SOLR-17304: PKG_VERSIONS not honored when loading the schema plugins

[AndreyBozhko]

https://issues.apache.org/jira/browse/SOLR-17304

Description

Right now, this is just a reproducer for the issue, and a quick attempt to fix it.

Steps to reproduce

1. Upload multiple versions of the same package to Solr (the versions don't have to be compatible with each other)
2. Upload a configset which requests a specific package version via PKG_VERSIONS in params.json
3. Create a collection using the uploaded configset

Expected behavior

• If the configsets references custom classes from the package, then Solr uses the package version constraint from params.json to initialize all the plugins mentioned in solrconfig.xml and schema.xml

Actual behavior

• Solr uses the package version specified in params.json only for loading the solrconfig.xml plugins
• For schema.xml plugins (field types, token filters, similarities, etc.) Solr always goes for the latest available package version, regardless of what was specified in params.json

Why though?

So it looks like when the schema.xml is loaded, and a SolrResourceLoader initializes all schema plugins, the loader isn't able to access the params.json.

The loader keeps a reference to the current SolrConfig which provides access to the params.json, but at the time the schema is loaded that reference is still null.

Checklist

Please review the following and check all that apply:

• [x] I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
• [x] I have created a Jira issue and added the issue ID to my pull request title.
• [x] I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
• [x] I have developed this patch against the main branch.
• [x] I have run ./gradlew check.
• [x] I have added tests for my changes.
• [ ] I have added documentation for the Reference Guide

[epugh]

It seems like if your test is passing, then you've fixed the bug!

The test does pass - but for completeness, I still want to check what happens when the collection is reloaded or when packages are refreshed, just to make sure nothing else breaks.

Awesome! When you are ready, if no one else moves this forward, please ping me.

[AndreyBozhko]

Hi @epugh, I've done another pass over this and I think the change is ready for review

[epugh]

@AndreyBozhko did you see the build failure?

[AndreyBozhko]

@AndreyBozhko did you see the build failure?

Hm, I'll try to rebase off of the tip of the main branch and see if I can reproduce the failure...

[AndreyBozhko]

Looks like the test failure is due to the latest commit 8c5ae1b to main.

@gerlowskija Could you take a look at this?

It seems like the "Convert /cluster filestore APIs to JAX-RS" change made it so that the responseHeader inside a V2Response is now a LinkedHashMap and not a NamedList, so invoking getResponseHeader or getStatus on the V2Response fails with a ClassCastException.

For the context - the test hits the /api/cluster/files/ endpoint to upload the file, and then tries to assert that response.getStatus() is 0.

>     java.lang.ClassCastException: class java.util.LinkedHashMap cannot be cast to class org.apache.solr.common.util.NamedList (java.util.LinkedHashMap is in module java.base of loader 'bootstrap'; org.apache.solr.common.util.NamedList is in unnamed module of loader 'app')
   >         at __randomizedtesting.SeedInfo.seed([E519C61B26401EA4:6B2A3A85909FE569]:0)
   >         at org.apache.solr.client.solrj.response.SolrResponseBase.getResponseHeader(SolrResponseBase.java:63)
   >         at org.apache.solr.client.solrj.response.SolrResponseBase.getStatus(SolrResponseBase.java:68)
   >         at org.apache.solr.pkg.PackageStoreSchemaPluginsTest.processRequest(PackageStoreSchemaPluginsTest.java:170)
   >         at org.apache.solr.pkg.PackageStoreSchemaPluginsTest.uploadPluginJar(PackageStoreSchemaPluginsTest.java:133)
   >         at org.apache.solr.pkg.PackageStoreSchemaPluginsTest.testCreateCollection_withPkgVersionsConstraint(PackageStoreSchemaPluginsTest.java:92)

[gerlowskija]

Thanks for flagging this @AndreyBozhko.

I'll take a look and try to fix the test in this PR tomorrow. Sorry to hold this up!

[epugh]

@AndreyBozhko give me a ping when this is syned with main!

[gerlowskija]

Hi @AndreyBozhko - sorry for the delay in unblocking this.

The reason your test started failing is that my recent commit subtly changed the "upload filestore" response in some cases. The response would still serialize to the same JSON or XML, but the "javabin" now parses a bit differently when received on the client side. This breaks a class/type expectation baked into the SolrResponseBase getters and triggers the ClassCastException you observed. This is a known issue right now with 'javabin' and our JAX-RS v2 APIs that we'll need to come up with a fix for at some point, if we decide to keep "javabin" in 10.0.

It seemed at first like something that'd be easy to workaround, but each thing I tried uncovered a different bug 😬

1. The standard workaround would be to ditch V2Request and instead use FileStoreApi.Upload, a SolrRequest implementation that's generated for us based on the v2 API definition. But this didn't work due to an issue with our generation template.
2. A solid second-tier workaround would be to modify V2Request usage to use a different response format (e.g. JSON or XML). But it turns out that the type/class assumptions made by SolrResponseBase.getStatus also break when JSON or XML are used! (Note that afaict this error happens any time JSON or XML are used - for any API, either v1 or v2, with or without my recent PR!)
3. Due to an embarrassing little coding error, v2 APIs respond with XML when wt=json is specified 😬 . I've filed a bug report for this here

Long story short, I intend to fix all these bugs, but I don't want to keep you blocked while I do. Will revert shortly and then you should be unblocked.

[gerlowskija]

I've reverted my commit on main to get this unblocked, and merged 'main' into the PR branch. The test passes for me locally now. Let me know if there's any other issues @AndreyBozhko / @epugh !

[AndreyBozhko]

Thanks @gerlowskija, appreciate you looking into this!