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 for uncountable plural names. #940

Closed
wants to merge 1 commit into from

Conversation

seanrucker
Copy link

The serializer complains if you try to use an uncountable plural name. E.g. "fish" is the same for both singular and plural.

This fix ensures the alias is only added once.

This PR also fixes the following issues:
#928
#1003

@tomdale
Copy link
Member

tomdale commented May 10, 2013

@seanrucker Looks great; happy to merge if we can get some tests to ensure we don't get any regressions on this in the future.

@seanrucker
Copy link
Author

@tomdale Simple tests have been added. Followed the same structure used to test person <=> people. What it comes down to is ensuring the serializer.pluralize and serializer.singularize can work with uncountables.

@seanrucker
Copy link
Author

@tomdale Please take a look also at the two other issues fixed by this PR (referenced in the description).

Fix for race condition.

We can't be certain this is the first time the alias has been pluralized. The JSONSerializer may have already pluralized it in the sideLoad -> configureSideloadMappingForType function as defaultSideloadRootForType returns the pluralized form.

Test serializer pluralize and singularize with uncountables.
@seanrucker
Copy link
Author

@igorT @tomdale Commits squashed as requested.

if (this._didPluralizeAliases) {
return;
} else {
this._didPluralizeAliases = true;

Choose a reason for hiding this comment

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

This should not happen until the end of the method. If the method encounters an exception while pluralizing aliases, then it would be inaccurate to mark the operation complete. Only after they've been pluralized successfully, should this be marked true

@seanrucker
Copy link
Author

Closing as this PR will take care of uncountables as well.#1098

@seanrucker seanrucker closed this Jul 26, 2013
@seanrucker seanrucker deleted the uncountables-fix branch July 26, 2013 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants