Skip to content

SOLR-18302: DAGP inter-module project() api promotions (part 2/3, draft)#4613

Draft
serhiy-bzhezytskyy wants to merge 7 commits into
apache:mainfrom
serhiy-bzhezytskyy:dagp-pr2-project-api
Draft

SOLR-18302: DAGP inter-module project() api promotions (part 2/3, draft)#4613
serhiy-bzhezytskyy wants to merge 7 commits into
apache:mainfrom
serhiy-bzhezytskyy:dagp-pr2-project-api

Conversation

@serhiy-bzhezytskyy

@serhiy-bzhezytskyy serhiy-bzhezytskyy commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Draft, for review — part 2 of 3. Builds on #4612 (which must merge first).

Applies DAGP's implementation -> api advice only for the ~36 inter-module project(...) dependencies where the type is genuinely part of the consuming module's public API — e.g. :solr:core publicly exposes :solr:solrj types, so api is correct and downstream modules compile against them.

The ~91 external-library api promotions DAGP also suggests are deliberately not here — see part 3/3 (draft, not for merge), which shows them for discussion. Per @dsmiley's point that DAGP is "api-happy" and liberal api is viral/transitive, those need case-by-case judgement.

gradlew check -x test passes.

AI-assisted, human-reviewed per AGENTS.md.

@serhiy-bzhezytskyy
SOLR-18296: add dependency-analysis (DAGP) reporting + apply safe advice
e933003 
Completes the dependency-analysis follow-up deferred from the Gradle 9 upgrade
(SOLR-18289), which dropped the Gradle-9-incompatible ca.cutterslade.analyze
plugin. Adds the Dependency Analysis Gradle Plugin (com.autonomousapps
build-health) configured to REPORT (warn), not fail — its advice is guidance
for human review, not enforcement.

Applies only the uncontroversial advice:
 - removal of genuinely-unused dependency declarations
 - a few test-scope corrections (implementation -> testImplementation)

Deliberately NOT included here (left for separate case-by-case review):
 - implementation -> api promotions (DAGP is aggressive here; per dev@ these
   need human judgement, as liberal api is viral/transitive)

Also forces kotlin-metadata-jvm to match Solr's Kotlin so DAGP can analyze the
:solr:ui (Compose/KMP) module. Lockfiles regenerated.
@serhiy-bzhezytskyy
SOLR-18296: add changelog entry for dependency-analysis PR (apache#4612)
03a76c4
@serhiy-bzhezytskyy serhiy-bzhezytskyy mentioned this pull request Jul 5, 2026
Draft
dsmiley
dsmiley reviewed Jul 7, 2026
View reviewed changes

@dsmiley dsmiley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(looked only at the suitable commit)
Looks good... I thought you'd make more changes.

@serhiy-bzhezytskyy
SOLR-18296: restore kafka-clients test jar as testRuntimeOnly (fix cr…
77e8ffa 
…oss-dc-manager tests)

DAGP flagged the kafka-clients test-classifier jar as an unused dependency, so the
earlier commit removed it. That was a false positive: no test compiles against it,
but EmbeddedKafkaCluster (from the kafka-streams test jar) loads
org.apache.kafka.test.TestCondition from it at runtime, so the *IntegrationTest
suites failed with ClassNotFoundException on CI.

Re-add it as testRuntimeOnly (runtime-only is the accurate scope, vs the
testImplementation it originally had) with a comment, and restore its license sha1.
A concrete example of why DAGP runs report-not-fail and its advice needs
case-by-case judgement.
@serhiy-bzhezytskyy serhiy-bzhezytskyy changed the title SOLR-18296: DAGP api promotions — inter-module project() deps (part 2/3, draft) SOLR-18302: DAGP inter-module project() api promotions (part 2/3, draft) Jul 7, 2026
@serhiy-bzhezytskyy
SOLR-18302: address review — revert api promotions, drop change-comme…
fa39a80 
…nts, fix changelog

Responding to review feedback on the safe-advice PR:

- Revert all external implementation->api promotions that had leaked in from the
  api-scope work (caffeine, zookeeper, jute, jsonpath, opentelemetry-sdk +
  exporter-prometheus in core; curator-client/jute in solrj-zookeeper; zookeeper
  +tests in test-framework; calcite in sql; cloud-storage in gcs; awssdk-s3 in
  s3). None are in the modules' public ABI and DAGP does not advise them; they
  belong in the separate api-scope discussion PRs, not here. Verified no
  downstream module relies on the former transitive leak.
- Revert solrj-jetty entirely: its changes were api promotions + manual cleanups,
  none part of the safe mechanical advice.
- Restore opentelemetry-sdk-metrics in test-framework: DAGP flagged it unused, but
  JettySolrRunner uses MetricReader from it and the prometheus exporter is
  declared transitive=false, so it is required (compile break otherwise).
- Restore the gjf-SOLR-18296 changelog entry that had been deleted by a rebase
  artifact; it belongs to the separate google-java-format change.
- Remove code comments that narrated the change/tool advice (per AGENTS.md); keep
  only terse why-comments matching existing style.
- Changelog: type changed->other; link to SOLR-18302 (the correct JIRA).
- Regenerate lockfiles/licenses.
@github-actions github-actions Bot removed documentation Improvements or additions to documentation cat:api labels Jul 7, 2026
@serhiy-bzhezytskyy
SOLR-18302: drop redundant self/duplicate test project deps in solrj-…
d0c3fde 
…jetty

Remove testImplementation project(':solr:solrj-jetty') (a module test-depending on
itself — the test source set already sees its own main classes) and the duplicate
testImplementation project(':solr:solrj') (already api project(':solr:solrj')). The
project(path: ':solr:solrj', configuration: 'testArtifacts') declaration stays; it
provides solrj's test classes, which is a real dependency. Verified :solr:solrj-jetty
test compiles and all 59 tests pass.
serhiy-bzhezytskyy added a commit to serhiy-bzhezytskyy/solr that referenced this pull request Jul 7, 2026
@serhiy-bzhezytskyy
SOLR-18302: [ILLUSTRATIVE, not for merge] external-library api promot…
6880181 
…ions (part 3/3)

Applies the 90 external-library implementation->api promotions DAGP advises across
the modules, for discussion. Verified all 90 against each module's ABI dump: every
one is genuinely exposed in public API (return type, parameter, field, or supertype)
today, regardless of the declaration -- e.g. CaffeineCache implements caffeine's
RemovalListener; ~82 core methods throw ZooKeeper's KeeperException. So this is an
api-hygiene question (is the exposure intended, or worth encapsulating?), not the
tool over-reaching.

jersey-core-server is left as implementation: DAGP advises api (it is ABI-exposed),
but its version comes from the implementation-scoped jersey BOM, so as api it fails
to resolve. A concrete case of advice that is correct about exposure yet cannot be
applied mechanically.

Builds on apache#4613. check -x test passes; lockfiles regenerated.
@serhiy-bzhezytskyy
SOLR-18302: address review — kotlin build script, drop self-test deps…
6306c81 
…, doc the lockfile filter

- Convert gradle/validation/dependency-analyze.gradle to Kotlin DSL (.gradle.kts),
  per the project's preference for Kotlin build files. Behavior is unchanged;
  buildHealth runs and all suppressions (:solr:ui, solrj-jetty, jwt-auth,
  report-not-fail) remain active.
- Remove the redundant self-referential testImplementation project(':self')
  declarations in api, solrj, solrj-streaming, solrj-zookeeper, and core (two were
  left-over 'cutterslade requires we state the obvious' workarounds for the plugin
  removed in SOLR-18289). Test source sets already see their own main classes.
- Add a one-line comment explaining why resolveAndLockAll skips the plugin's
  synthetic dagp.* configurations.
- Regenerate the changelog entry with the writeChangelog task so it matches the
  documented format.
@serhiy-bzhezytskyy
SOLR-18296: promote genuinely-public inter-module deps to api (part 2/3)
fa4f0f0 
Applies DAGP's api advice ONLY for inter-module project() dependencies that are
genuinely part of the consuming module's public API (e.g. solr-core's public
types expose solrj), where 'api' is correct so downstream modules compile.

External-library api promotions are intentionally excluded here (see part 3/3);
per dev@ feedback those need case-by-case judgement rather than blanket promotion.

Builds on apache#4612. Lockfiles regenerated.
serhiy-bzhezytskyy added a commit to serhiy-bzhezytskyy/solr that referenced this pull request Jul 7, 2026
@serhiy-bzhezytskyy
SOLR-18302: [ILLUSTRATIVE, not for merge] external-library api promot…
57c861b 
…ions (part 3/3)

Applies the 90 external-library implementation->api promotions DAGP advises across
the modules, for discussion. Verified all 90 against each module's ABI dump: every
one is genuinely exposed in public API (return type, parameter, field, or supertype)
today, regardless of the declaration -- e.g. CaffeineCache implements caffeine's
RemovalListener; ~82 core methods throw ZooKeeper's KeeperException. So this is an
api-hygiene question (is the exposure intended, or worth encapsulating?), not the
tool over-reaching.

jersey-core-server is left as implementation: DAGP advises api (it is ABI-exposed),
but its version comes from the implementation-scoped jersey BOM, so as api it fails
to resolve. A concrete case of advice that is correct about exposure yet cannot be
applied mechanically.

Builds on apache#4613. check -x test passes; lockfiles regenerated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dsmiley dsmiley dsmiley left review comments

Assignees

No one assigned

Labels

admin-ui cat:api client:solrj dependencies Dependency upgrades module:clustering module:extraction module:gcs-repository module:jwt-auth module:langid module:ltr module:opentelemetry module:s3-repository module:scripting module:sql test-framework tool:build

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@serhiy-bzhezytskyy @dsmiley