Repository navigation
fix(emails): copy list and map arguments in CreateEmailOptions builder - #150
Merged
kewynakshlley merged 2 commits intoOct 7, 2026
Merged
kewynakshlley merged 2 commits into
kewynakshlley merged 2 commits into
Conversation
kewynakshlley
approved these changes
Oct 7, 2026
Collaborator
|
Thanks for contributing @RaphaelFakhri |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Copies the collection arguments that
CreateEmailOptions.Builderreceives, so the builder no longer keeps a reference to the caller's list or map.Problem
to(List<String>),tags(List<Tag>),attachments(List<Attachment>)andheaders(Map<String, String>)store the argument as-is. TheaddTo,addTag,addAttachmentandaddHeadermethods then mutate that same object:With an immutable collection, the follow-up call throws
UnsupportedOperationException:With a mutable collection,
addTochanges the caller's list, and later changes to the caller's list change the built options.cc(List),bcc(List)andreplyTo(List)already copy the argument, so the four methods behave differently from their siblings.Solution
Store a copy in each of the four methods. The methods still replace the current value, as before, and
nullstill clears it.Tests
CreateEmailOptionsTestcovers each method with an immutable argument followed by anadd*call, and checks thatto(List)does not write through to the caller's list. All five tests fail without the change and pass with it.The full test task passes.
Summary by cubic
Fixes
CreateEmailOptions.Builderstoring the caller'sto,tags,attachments, andheaderscollections by reference, so the builder now copies them instead.Previously,
addTo,addTag,addAttachment, andaddHeadermutated the caller's collection: an immutable list such asList.of(...)causedUnsupportedOperationException, and a mutable list changed the caller's data.cc,bcc, andreplyToalready copied their arguments, so the methods behaved inconsistently.Adds
CreateEmailOptionsTest, which covers immutable arguments followed byadd*calls and confirmsto(List)doesn't write through to the caller's list.Written for commit dd70a94. Summary will update on new commits.