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

Use ArrayList for storing parameters locally #135

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

mtughan
Copy link
Contributor

@mtughan mtughan commented Dec 10, 2024

Instead of using the Java 11-native List.of and List.copyOf to store a copy of the parameters list locally, which aren't natively supported by XStream serialization, use ArrayList, which is natively supported. When returning the parameters, wrap them in an unmodifiable list to avoid unexpected external modifications.

This unfortunately means that this class is not "immutable" anymore as the parameters can technically be changed, but the current code does not provide a way to accomplish that.

This PR stems from a discussion on #134.

Testing done

Verified using existing unit tests plus local testing to see how the class is serialized out to disk.

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

Instead of using the Java 11-native `List.of` and `List.copyOf` to store
a copy of the parameters list locally, which aren't natively supported
by XStream serialization, use `ArrayList`, which is natively supported.
When returning the parameters, wrap them in an unmodifiable list to
avoid unexpected external modifications.

This unfortunately means that this class is not "immutable" anymore as
the parameters can technically be changed, but the current code does not
provide a way to accomplish that.
@mtughan mtughan self-assigned this Dec 10, 2024
@mtughan mtughan added the bugfix label Dec 10, 2024
@mtughan mtughan merged commit c44e60d into jenkinsci:main Dec 10, 2024
17 checks passed
@mtughan mtughan deleted the use-arraylist branch December 10, 2024 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant