Skip to content

Fixes the formatting for sealed interfaces in the JavaSpring generator #21513

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

Conversation

Mattias-Sehlstedt
Copy link
Contributor

@alex-nt recently added some great functionality to support sealed interfaces. I was looking into adding the same for the Spring clients (RestClient and WebClient) but noticed some minor formatting issue and that final was sometimes set on a class even if wasn't part of a sealed structure.

It would be great if Alex could review the suggested changes and comment on whether they make sense or not.

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    (For Windows users, please run the script in WSL)
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • File the PR against the correct branch: master (upcoming 7.x.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@cachescrubber (2022/02) @welshm (2022/02) @MelleD (2022/02) @atextor (2022/02) @manedev79 (2022/02) @javisst (2022/02) @borsch (2022/02) @banlevente (2022/02) @Zomzog (2022/09) @martin-mfg (2023/08)

@alex-nt
Copy link
Contributor

alex-nt commented Jul 3, 2025

My view at the time was that if you select sealed, it will fully seal everything. So everything in the model becomes either sealed or final so that nobody can inherit/do weird stuff with it. For me it was a feature to not allow any change to the generated model 😅. IDK if it was the right approach though.

@Mattias-Sehlstedt
Copy link
Contributor Author

Mr. Porco flies in and responds instantly!

In my experience it is generally best to not put final on classes unless it is necessary (same as people wanting most methods to be public so that they can be overridden), since sometimes one will want to extend the classes in order to adjust some representation (e.g., changing their toString()).

But I guess it then comes down to what the project want useSealed to imply. Is it merely in the context of oneOf-interfaces or does it also affect all classes? (I am interested in knowing since I am planning on replicating the setting for Spring-clients).

@Mattias-Sehlstedt
Copy link
Contributor Author

I would personally like to have the option to have the sealed structure only apply for interfaces, since what I am after is being able to conduct strongly-typed switch-cases (And I do not want to restrict all other classes). So I am wondering what would be the best way to approach that without making a templating nightmare with useSealedModel, useSealedInterface, useSealedEverything...

Maybe an internally hidden additionalProperty similar to how staticRequest was introduced here? So we would allow useOneOfInterfaces to be set to sealed and that would apply your changes but only to interfaces. And useSealed would remain what it currently is?

@alex-nt
Copy link
Contributor

alex-nt commented Jul 3, 2025

My bad naming might have put us in a slight corner as now I've set the expectation that useSealed seals everything. We might need to add a hidden property to set the levels of sealing?!?.

@Mattias-Sehlstedt
Copy link
Contributor Author

I have introduced a separate PR where I add samples for the useSealed setting (i.e., this PR but omitting the change to removing final).

I will investigate how to best introduce so that the configuration settings can offer both a setting for where only interfaces are sealed but also a setting for where everything is sealed (i.e., keeping the currently useSealed behavior).

@Mattias-Sehlstedt Mattias-Sehlstedt marked this pull request as draft July 4, 2025 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants