Repository navigation
[COLLECTIONS-899] Convert supplementary characters in CaseInsensitiveMap keys - #749
Open
sharanggupta wants to merge 1 commit into
Open
sharanggupta wants to merge 1 commit into
sharanggupta wants to merge 1 commit into
Conversation
…Map keys convertKey() converted each UTF-16 char, so characters outside the Basic Multilingual Plane were never converted and, for example, U+10400 and U+10428 stayed two keys. Convert each code point instead; strings in the Basic Multilingual Plane convert as before. Describe the conversion in the Javadoc: the simple Unicode case mappings Character.toLowerCase(Character.toUpperCase(codePoint)), which are not the same as String.toLowerCase(Locale).
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.
Fixes COLLECTIONS-899, following the discussion there.
What
As discussed in the issue,
CaseInsensitiveMapdeliberately converts each character withCharacter.toLowerCase(Character.toUpperCase(c))since COLLECTIONS-294, which avoids the locale problems ofString.toLowerCase(). Two things did not match that, though:String.toLowerCase(). As the report shows, U+03D0 (GREEK BETA SYMBOL) and U+03B2 (GREEK SMALL LETTER BETA) give the same key, while"\u03D0".toLowerCase(Locale.ROOT)leaves U+03D0 unchanged.convertKey()works onchars, so characters outside the Basic Multilingual Plane, which are surrogate pairs, are never converted. U+10400 and U+10428 (DESERET CAPITAL and SMALL LETTER LONG I) stay two different keys. On Java 17 and later,String.equalsIgnoreCase()compares code points and considers them equal (on Java 8 and 11 it does not).How
convertKey()now converts each code point withCharacter.toLowerCase(Character.toUpperCase(int))and appends it withStringBuilder.appendCodePoint(), so supplementary characters are converted like any other character. The result for strings in the Basic Multilingual Plane is unchanged, and unpaired surrogates are kept as they are, as before.convertKey()now say what the conversion does: the simple, one-to-one Unicode case mappings, independently of the locale, which is not the same asString.toLowerCase(Locale). The example from the report is included.A map serialized by an older version that holds two keys differing only in the case of a supplementary character gets one entry for them when it is deserialized, since
doReadObject()puts every entry again.Tests
In
CaseInsensitiveMapTest:testSupplementaryCharactersputs keys with the Deseret capital letter, alone and inside a longer string. It gets them with the small letter and checks the stored lowercase keys. It checks that both spellings are one entry, also after serialization, and that a different Deseret letter is another entry. It fails on master (expected: <value> but was: <null>) and passes with this change.testUncasedSupplementaryCharactersAndUnpairedSurrogateschecks that an emoji and unpaired surrogates are kept as they are.testConversionIsIdempotentchecks, for every code point, that a converted key converts to itself, which deserialization relies on, since it converts the keys again.The tests do not depend on the Java version (
equalsIgnoreCase()differs between Java 8/11 and 17+).CaseInsensitiveMapTestpasses on Java 8 and 17, and the default Maven goal passes locally on Java 17.Before you push a pull request, review this list:
mvn; that'smvnon the command line by itself.