adityamparikh opened a new pull request, #133:
URL: https://github.com/apache/solr-mcp/pull/133
## Summary
Extends JSpecify null-safety enforcement from the single root package
`org.apache.solr.mcp.server` to **every sub-package** in the codebase — main
and test alike.
Before this change, `NullAway:OnlyNullMarked=true` was silently skipping all
production code except the (mostly empty) root package. So the project
advertised null-safety but only enforced it on `Main.java`.
## Changes
1. **`package-info.java` with `@NullMarked` added to 10 packages** (8 main +
2 test-only):
- main: `.collection`, `.config`, `.indexing`,
`.indexing.documentcreator`, `.metadata`, `.search`, `.security`, `.util`
- test-only: `.containerization`, `.observability`
2. **Production code annotated with `@Nullable` where appropriate**, derived
from running NullAway:
- `CollectionUtils.getLong/getFloat/getInteger` — return null on
missing/unparseable
- `CollectionService` cache/handler metrics methods — return null when
the Solr endpoint is unavailable (graceful degradation by design)
- Many `Dtos.java` record components:
`SolrMetrics.{cacheStats,handlerStats}`, all of
`CacheStats`/`CacheInfo`/`HandlerStats`/`HandlerInfo`,
`SolrHealthStatus.{errorMessage,responseTime,totalDocuments,solrVersion,status}`,
`IndexStats.{numDocs,segmentCount}`
- `@McpToolParam(required = false)` parameters on
`CollectionService.createCollection` and `SearchService.search`
- `JsonResponseParser.convertValue` — JSON null inputs
- `CollectionService.isCacheStatsEmpty` — accepts @Nullable
3. **`HttpSecurityConfiguration` Spring `@Value` fields initialised with
defaults** (`""` for `issuerUrl` matching the property default, `List.of()` for
`allowedOrigins`) so the analyser can see they are always non-null after
construction.
4. **NullAway is explicitly disabled on `compileTestJava` for this PR**
(one-line in `build.gradle.kts` with explanatory comment).
- The rollout exposes ~30 test sites that unbox or dereference values now
declared `@Nullable` in production (Map.get returns, optional metric fields,
etc.).
- Each can be tightened by extracting to a local + `assertNotNull`, but
the volume makes test-side cleanup a deliberate follow-up.
- **Production code is fully enforced.**
## Why draft
Draft because the test-side cleanup is intentionally deferred. Two
reasonable paths forward:
- **Merge as-is**, then a follow-up PR fixes each test site (`@Nullable`
flow analysis requires extracting to locals + `assertNotNull`). Roughly 30
sites across `CollectionServiceTest`, `CollectionServiceIntegrationTest`,
`CollectionUtilsTest`, `SearchServiceIntegrationTest`,
`ConferenceEndToEndIntegrationTest`, `IndexingServiceIntegrationTest`,
`XmlIndexingTest`, `SchemaServiceTest`, `SchemaServiceIntegrationTest`,
`TraceAssertions`, `DistributedTracingTest`.
- **Land both at once**, by extending this PR with the test-side cleanups
before un-drafting.
Happy to do either.
## Test plan
- [x] `./gradlew build` — full build green (compile + unit + integration)
- [x] NullAway enforces null safety on all production code
- [x] Pre-existing tests still pass at runtime
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
