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

Add some missing validation to property getters and setters #5952

Merged
merged 1 commit into from
Oct 19, 2018

Conversation

tgoyne
Copy link
Member

@tgoyne tgoyne commented Oct 9, 2018

6c02999 shifted around where the checks for valid threads and objects were done, and a few turned out to be wrong. This resulted in either crashing within core rather than throwing an exception, or throwing an untranslated C++ exception rather than the appropriate obj-c exception.

RLMArray and RLMResults already have tests similar to the ones added here.

@tgoyne tgoyne self-assigned this Oct 9, 2018
@bmunkholm
Copy link
Contributor

@tgoyne Was issues reported for this? Then we should list them in the Release note instead of this PR that I added.

@tgoyne
Copy link
Member Author

tgoyne commented Oct 10, 2018

No, I noticed it myself while writing some other code.

6c02999 shifted around where the checks for
valid threads and objects were done, and a few turned out to be wrong. This
resulted in either crashing within core rather than throwing an exception, or
throwing an untranslated C++ exception rather than the appropriate obj-c
exception.
@tgoyne tgoyne force-pushed the tg/deleted-object-validation branch 2 times, most recently from 59392fa to ebecc65 Compare October 19, 2018 20:35
@tgoyne tgoyne merged commit b0b6de9 into master Oct 19, 2018
@tgoyne tgoyne deleted the tg/deleted-object-validation branch October 19, 2018 21:12
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants