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

WW-5352 Ensure StrutsParameter compatibility with TypeConversion setters #865

Closed
wants to merge 1 commit into from

Conversation

kusalk
Copy link
Member

@kusalk kusalk commented Jan 29, 2024

Copy link

sonarcloud bot commented Jan 29, 2024

@kusalk
Copy link
Member Author

kusalk commented Jan 29, 2024

@lukaszlenart

I was hoping you could clear up some confusion of mine as I'm not well versed in how the Struts TypeConversion functionality behaves. I'm trying to understand whether it is possible to set a collection of Model class objects on an Action where only a setter exists?

For example - for the following action,

public class ExampleAction {
    // Configured with some TypeConverter
    public void setModels(List<Model> models) {
        this.models = models;
    }

    public static class Model {
        public String name;
    }
}

is it possible to configure a TypeConverter that would allow the following request to execute as intended?
example.action?models[0].name=kusal&models[1].name=lukasz

If so, I may need to tweak the StrutsParameter mechanism to also recognise annotations on Setters (which is what this draft PR does).

@lukaszlenart
Copy link
Member

This should work without a converter, the Model has the default constructor and Struts uses OGNL logic to create and extend lists

@kusalk
Copy link
Member Author

kusalk commented Jan 29, 2024

So I tried testing this, and it seems without a matching getter, the list will not be populated. It will simply be set to an empty list.

The current @StrutsParameter logic only checks for annotations on getters when trying to set a collection or model class. My concern was that it might be possible to do this with only a setter. But if not, we can safely close this PR.

@kusalk
Copy link
Member Author

kusalk commented Jan 29, 2024

I've just had a read of the following TypeConverter test and it seems it is only possible to set a List without a matching getter when using a serialized format.
core/src/test/java/com/opensymphony/xwork2/test/annotations/PersonActionTest.java

So in the earlier example I gave, the request would have to be a custom format as defined in the type converter, e.g.:
example.action?models=kusal:lukasz

So I think we are indeed okay to close this PR.

@kusalk kusalk closed this Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants