Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LinkedMultiValueMap downstream issue with MultiValueMapAdapter package private base class #25960

Closed
michaelhaessig opened this issue Oct 23, 2020 · 4 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: regression A bug that is also a regression
Milestone

Comments

@michaelhaessig
Copy link

Affects: > 5.2.9.RELEASE


Hi spring team,

i've run into an issue that is caused by a recent change in the MultiValueMap class hierarchy.

The changes introduced in the commits bec89db added a new private class MultiValueMapAdapter and LinkedMultiValueMap now inherits form this class.

Background on how LinkedMultiValueMap is implicitly used:
We cache specific spring mvc controller responses, some controller methods declare the following

@MatrixVariable(pathVar = "filter") Map<String, List<String>> filters

Internally spring creates LinkedMultiValueMap instances for those filters.

The new class hierarchy change caused the following downstream issue:
We are using a distributed hazelcast cache cluster, this cluster holds objects that contain LinkedMultiValueMap objects.
The hazelcast nodes do not have the spring jar on the classpath, we use the User code deployment feature that deploys specific java classes to the cluster to support storing those Map instances.

Our config looked like this:

    <user-code-deployment enabled="true">
        <classNames>
            <!-- CacheKey data can contain the following classes -->
            <className>org.springframework.util.MultiValueMap</className>
            <className>org.springframework.util.LinkedMultiValueMap</className>
        </classNames>
    </user-code-deployment>

With the change i've added the new class:

   <className>org.springframework.util.MultiValueMapAdapter</className>

Now we are getting the following exception:

java.lang.IllegalAccessError: class org.springframework.util.LinkedMultiValueMap cannot access its superclass org.springframework.util.MultiValueMapAdapter

The exception is thrown in the code deployment execution, reason being that the new parent class MultiValueMapAdapter has package private access.

Temporary fix:
I was able to solve the issue by re declaring the MultiValueMapAdapter class in my project code with public access.
This probably only works because of the class loader execution order.

Request:
The simplest solution would be to make the MultiValueMapAdapter class public.
I totally understand that the class is not public for a reason, but to respect backwards compatibility i would only suggest that this should be considered. I just want to mention the problem in case someone else runs into the same issue.

Another solution on our side would be adding the spring-core.jar to the hazelcast cluster classpath, but this would force us to make a custom build/config for the hazelcast nodes.

If there is another solution i would appreciate to get any input on this.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 23, 2020
@jhoeller jhoeller self-assigned this Oct 23, 2020
@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 23, 2020
@jhoeller jhoeller added this to the 5.2.10 milestone Oct 23, 2020
@jhoeller
Copy link
Contributor

jhoeller commented Oct 23, 2020

We could make MultiValueMapAdapter public indeed, or go with a duplicated implementation as in the 4.3.x line. In any case, avoiding a non-public base class of LinkedMultiValueMap.

I'll see what we can do for 5.2.10 and 5.1.19 there, and also for 5.0.20 (since they all received the same original fix for #25140).

@jhoeller jhoeller changed the title CollectionUtils new MultiValueMapAdapter package private downstream issue LinkedMultiValueMap downstream issue with MultiValueMapAdapter package private base class Oct 23, 2020
@jhoeller jhoeller added the for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x label Oct 23, 2020
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x labels Oct 23, 2020
@jhoeller jhoeller added for: backport-to-5.0.x and removed status: backported An issue that has been backported to maintenance branches labels Oct 23, 2020
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.0.x labels Oct 23, 2020
@michaelhaessig
Copy link
Author

That would be great, thanks for the fast reply.

jhoeller added a commit that referenced this issue Oct 26, 2020
jhoeller added a commit that referenced this issue Oct 26, 2020
@jhoeller
Copy link
Contributor

I went with making MultiValueMapAdapter public (and available for direct usage, not least of it all as base class for custom MultiValueMap implementations) in 5.3, while restoring an independent implementation for LinkedMultiValueMap (without a base class, simply duplicating the code locally) in 5.2.10, 5.1.19 and 5.0.20.

@michaelhaessig
Copy link
Author

Perfect! thanks for the quick resolution, will test it out once 5.2.10 is available.

zx20110729 pushed a commit to zx20110729/spring-framework that referenced this issue Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

3 participants