adityamparikh commented on PR #86:
URL: https://github.com/apache/solr-mcp/pull/86#issuecomment-5149295812
Heads-up: this branch currently shows as conflicting against `main`, but the
conflict surface is much smaller than the 53-commit gap suggests. I rebased it
locally onto `main` (`a57f4d9`) to check, and it comes out clean — sharing what
I found in case it saves you time.
**The two conflicts, and how they resolve:**
1. `config/SolrConfig.java` — import-block collision only. `main` added
`org.springframework.util.StringUtils`; this branch added
`org.springframework.web.client.RestClient`. Keep both.
2. `McpToolRegistrationTest.java` — `SchemaService` moved from
`org.apache.solr.mcp.server.metadata` to `org.apache.solr.mcp.server.schema` on
`main`. Switch the import to the new package and keep the
`refguide.RefGuideService` one.
That's the whole manual resolution — two import lines.
**The `metadata` → `schema` package split doesn't bite.** It was a pure
rename in git history and this branch never touched those files, so git replays
it automatically. No duplicate `SchemaService`, no stale `metadata/` package
left behind.
**Result after rebase:** `+293 / -0` across the 4 files, and `./gradlew
build` is green — 359 tests, 0 failures, 0 errors (`RefGuideServiceTest` 6/6).
`RefGuideService` is self-contained enough (Spring + MCP annotations +
`RestClient`) that none of the upstream churn reached it.
**One thing that will block the native path:** `RefGuideServiceTest` is
Mockito-based (`@ExtendWith(MockitoExtension.class)`), but doesn't carry
`@DisabledInNativeImage`. Every other Mockito test in the repo does, because
ByteBuddy proxies don't survive GraalVM's closed-world assumption — so
`./gradlew nativeTest -Pnative` will fail on it. One-line fix:
```java
import org.junit.jupiter.api.condition.DisabledInNativeImage;
@DisabledInNativeImage
@ExtendWith(MockitoExtension.class)
class RefGuideServiceTest {
```
Two smaller review notes, take or leave:
- The `RestClient` `@Bean` is added to `SolrConfig`, which is otherwise
scoped to SolrJ client wiring. It works, but it might sit more naturally in the
`refguide` package's own config.
- `getRefGuideUrl` fetches `https://solr.apache.org/guide/sitemap.xml` on
every invocation, with no caching and no explicit timeout. Might be worth a
cache and a connect/read timeout given it's a remote call on a hot path.
Happy to send the rebased commits as a patch if that's useful — they
preserve your authorship and sign-off.
--
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]
