adityamparikh opened a new pull request, #104:
URL: https://github.com/apache/solr-mcp/pull/104
> **Reworked.** The earlier revision added `spring-boot-starter-validation`
(Hibernate Validator, not otherwise on the classpath), a custom `@SolrUrl`
constraint annotation plus its `ConstraintValidator`, `@Validated` on the
record, and, in a stray commit, deleted `logback.xml` — which #189 had since
made load-bearing and #193 has now removed the right way. This revision keeps
the intent and drops all of that: the check lives in the record's compact
constructor, with no new dependency, no annotations and no extra classes.
## Problem
`solr.url` is bound into a record and handed to SolrJ. Nothing checks that
it is a URL SolrJ can actually use, so a misconfigured deployment starts
cleanly and fails at first request with an opaque client error. The easy
mistake is omitting the scheme:
```
solr.url=localhost:8983
→ URI.create(...) parses scheme="localhost", host=null
→ SolrConfig normalizes the *path* by string concatenation, never noticing
→ SolrJ: "localhost:8983/solr/" is not a URL it can connect to
```
## Change
`SolrConfigurationProperties` gets a compact constructor that rejects
anything but an **absolute `http`/`https` URL with a host**, with a message
that names the property, shows an example and echoes the offending value:
| Accepted | Rejected |
|---|---|
| `http://localhost:8983` | `localhost:8983` (scheme `localhost`, no host) |
| `http://localhost:8983/solr/` | `solr.example.com`, `/solr` (not absolute)
|
| `https://solr.internal:8983/custom/solr/` | `ftp://…`, `file://…` (SolrJ
cannot speak them) |
| `http://solr:8983/solr/` | `not a url`, `http://`, empty, blank, absent |
Boot surfaces the `IllegalArgumentException` through its bind failure
analyzer, so startup prints *Failed to bind properties under 'solr'* with the
reason, rather than a stack trace at first request.
**Why the constructor and not Bean Validation.** `@Validated` + constraints
is the Boot idiom when Hibernate Validator is already present. Here it is not:
adding it for one field means a new runtime dependency in the native image plus
two extra classes for a custom constraint, and `@NotBlank`/`@URL` alone cannot
express "http(s) with a host" (Hibernate's `@URL` accepts `ftp://` and
`file://`). A record's compact constructor is the plain-Java place for an
invariant, and it is also where JSpecify's boundary argument applies: the
binder invokes it reflectively and will pass `null` when the property is
absent, so the one `hasText` check here is the guard for that boundary, not a
redundant one.
Path normalization (appending `/solr/`) is unchanged and still lives in
`SolrConfig`.
## Verification
- **Test first.** `SolrConfigurationPropertiesTest` (19 cases: 7 accepted
URLs, 9 rejected values, absent value, and two `ApplicationContextRunner` bind
tests) was written before the change and run against `main`: all 11 rejection
cases fail, all acceptance cases pass. The two context-runner tests are
`@DisabledInNativeImage` because that runner builds its context through a JDK
dynamic proxy; the constructor itself is plain Java and runs natively.
- Existing `SolrConfigUrlNormalizationTest` and `SolrConfigAuthTest`
unchanged and green.
- `./gradlew build` on JDK 25 against `main` @ `b4ffe18`, run twice. First
run: green. Second run, in a clean worktree: 6 failures, all
`DistributedTracingTest`, root cause `SolrClientUtilsException: Http Call
Status: 404` while Testcontainers started `solr:9.9-slim` — a container startup
flake unrelated to this change; the class passes 6/6 on rerun in the same
worktree.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01Wh7SJkZhL1uuK7pYc3SLk8
--
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]
