adityamparikh commented on PR #108: URL: https://github.com/apache/solr-mcp/pull/108#issuecomment-5699600823
Closing this. Re-checked against `main` @ `7d920af`, and most of the rationale in the description doesn't hold up: - **"a null collection silently targets the client's *default* collection"** — not true here. `SolrConfig.buildSolrClient` never calls `withDefaultCollection`, so there is no default to fall back to. - **whitespace needs a guard** — it doesn't. `" "` encodes to `%20`, reaches Solr and 404s, and `SearchService.withRemediationHint` already turns that into the `list-collections` hint. Same for any unknown collection name. What's actually left is much narrower: only the *empty* string degrades. `/solr//select` is rejected by Jetty during URI parsing, so it is a **400 `Ambiguous URI empty segment`** rather than a 404 — it misses the `NOT_FOUND` branch, carries none of the message tokens the other branches match on, and reaches the client raw. | collection argument | HTTP | what the client sees | |---|---|---| | `""` | 400 | `Ambiguous URI empty segment` | | `" "` | 404 | + `list-collections` hint | | unknown name | 404 | + `list-collections` hint | That is a real dead end, but a rare one, and not worth the shape this PR has taken: eleven call sites plus ~140 lines of Mockito tests that only exercise the shared helper through four service constructors, and add ten `@DisabledInNativeImage` skips to `nativeTest`. If it proves worth fixing later, the cheaper place is `SolrClient.request(SolrRequest<?>, String)` — the single abstract funnel every operation goes through, and where `null` legitimately means "no collection targeted" while blank never is. One check there would cover every existing and future tool. The one piece worth keeping is the de-duplication: the same message exists twice, as `CollectionService.BLANK_COLLECTION_NAME_ERROR` and as a private `SchemaService.requireCollection`. I'll fold that into the next PR that touches `SchemaService`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
