Skip to content

Increase ClientOptions mutability safety by cloning objects - #827

Merged
qsdigor merged 15 commits into
integration/version-2from
feature/771-increase-mutability-safety-by-cloning-objects
Aug 1, 2022
Merged

Increase ClientOptions mutability safety by cloning objects#827
qsdigor merged 15 commits into
integration/version-2from
feature/771-increase-mutability-safety-by-cloning-objects

Conversation

@qsdigor

@qsdigorqsdigor commented Jul 20, 2022

Copy link
Copy Markdown
Contributor

Added ObjectCopyUtil with copy method for ClientOptions. Changing fields in options after it is passed to AblyRealtime will no longer change options in the AblyRealtime instance.

QSD_igorand others added 7 commits July 18, 2022 13:25
@qsdigorqsdigor self-assigned this Jul 20, 2022
@qsdigor
qsdigor marked this pull request as ready for review July 20, 2022 14:16
@qsdigorqsdigor linked an issue Jul 20, 2022 that may be closed by this pull request
Increase mutability safety by cloning objects #771
Open
KacperKluka
KacperKluka suggested changes Jul 21, 2022
View reviewed changes

@KacperKlukaKacperKluka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few questions and suggestions 😉

Comment threadcore/src/test/java/io/ably/lib/test/realtime/RealtimeAuthTest.java Outdated
Comment threadcore/src/main/java/io/ably/lib/rest/Auth.java
Comment threadcore/src/main/java/io/ably/lib/rest/Auth.java
Comment threadcore/src/main/java/io/ably/lib/rest/AblyBase.java
Comment threadcore/src/main/java/io/ably/lib/util/ObjectCopyUtil.java Outdated
Comment threadcore/src/main/java/io/ably/lib/util/ObjectCopyUtil.java Outdated
@qsdigor
qsdigor requested a review from KacperKlukaJuly 22, 2022 07:37
ikbalkaya
ikbalkaya suggested changes Jul 25, 2022
View reviewed changes

@ikbalkayaikbalkaya left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have made some suggestions

Comment threadcore/src/main/java/io/ably/lib/util/ObjectCopyUtil.java Outdated
Comment threadcore/src/main/java/io/ably/lib/util/ObjectCopyUtil.java Outdated
Comment threadcore/src/test/java/io/ably/lib/test/realtime/RealtimeAuthTest.java Outdated
Comment threadcore/src/test/java/io/ably/lib/test/realtime/RealtimeAuthTest.java Outdated
Comment threadcore/src/test/java/io/ably/lib/test/realtime/RealtimeAuthTest.java Outdated
@qsdigor@ikbalkaya
Update core/src/main/java/io/ably/lib/util/ObjectCopyUtil.java
66c5a5c
Co-authored-by: Ikbal Kaya <ceikbal@gmail.com>
@qsdigor
qsdigor requested a review from ikbalkayaJuly 25, 2022 13:01
ikbalkaya
ikbalkaya reviewed Jul 26, 2022
View reviewed changes
Comment threadcore/src/main/java/io/ably/lib/util/ObjectCopyUtil.java Outdated
@qsdigor
qsdigor requested a review from ikbalkayaJuly 26, 2022 12:55
@qsdigorqsdigor changed the title Feature/771 increase mutability safety by cloning objectsIncrease ClientOptions mutability safety by cloning objectsJul 27, 2022
KacperKluka
KacperKluka approved these changes Jul 27, 2022
View reviewed changes

@KacperKlukaKacperKluka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@qsdigor
qsdigor merged commit ca81dea into integration/version-2Aug 1, 2022
@qsdigor
qsdigor deleted the feature/771-increase-mutability-safety-by-cloning-objects branch August 1, 2022 13:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@QuintinWillisonQuintinWillisonAwaiting requested review from QuintinWillison
2 more reviewers
@ikbalkayaikbalkayaikbalkaya approved these changes
@KacperKlukaKacperKlukaKacperKluka approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@qsdigorqsdigor

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Increase mutability safety by cloning objects

4 participants

@qsdigor@ikbalkaya@QuintinWillison@KacperKluka