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

[fix][java-client] Returns immutable data set when use TableView. #14833

Merged
merged 3 commits into from
Mar 24, 2022

Conversation

Technoboy-
Copy link
Contributor

@Technoboy- Technoboy- commented Mar 24, 2022

Fixes #14821

Master Issue: #14821

Motivation

Currently TableViewImpl methods that access the underlying map, e.g., TableViewImpl::KeySet() (https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/TableViewImpl.java#L141) returns views of the underlying map, which means users could modify the returned value and accidentally change the internal state of TableViewImpl.

It's better to return an immutable set to users.

Documentation

  • no-need-doc

@Technoboy- Technoboy- self-assigned this Mar 24, 2022
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 24, 2022
@mattisonchao
Copy link
Member

mattisonchao commented Mar 24, 2022

@Technoboy-

Can we use Collections.unmodifiableSet() to return an immutable Set?
I'm not very deep in TableView , please let me know what you think if there are any mistakes.
Thanks!

@Technoboy-
Copy link
Contributor Author

Technoboy- commented Mar 24, 2022

@Technoboy-

Can we use Collections.unmodifiableSet() to return an immutable Set? I'm not very deep in TableView , please let me know what you think if there are any mistakes. Thanks!

My first version is using immutable set like Collections.unmodifiableSet(). But users want to modify the data, and after talking with @nlu90, it's good to return a new data set for them.

@Technoboy-
Copy link
Contributor Author

@Technoboy-
Can we use Collections.unmodifiableSet() to return an immutable Set? I'm not very deep in TableView , please let me know what you think if there are any mistakes. Thanks!

My first version is using immutable set like Collections.unmodifiableSet(). But users want to modify the data, and after talking with @nlu90, it's good to return a new data set for them.

Change to immutable set

@Technoboy- Technoboy- changed the title Returns the new data set when use TableView. Returns immutable data set when use TableView. Mar 24, 2022
@codelipenghui codelipenghui added this to the 2.11.0 milestone Mar 24, 2022
@codelipenghui codelipenghui added type/bug The PR fixed a bug or issue reported a bug component/client-java labels Mar 24, 2022
@codelipenghui codelipenghui changed the title Returns immutable data set when use TableView. [fix][java-client] Returns immutable data set when use TableView. Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.10 doc-not-needed Your PR changes do not impact docs release/2.10.1 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TableViewImpl leaking internal state via accessors
5 participants