Skip to content

[COLLECTIONS-899] Convert supplementary characters in CaseInsensitiveMap keys - #749

Open
sharanggupta wants to merge 1 commit into
apache:masterfrom
sharanggupta:COLLECTIONS-899-supplementary-case
Open

sharanggupta wants to merge 1 commit into
apache:masterfrom
sharanggupta:COLLECTIONS-899-supplementary-case

Conversation

@sharanggupta

Copy link
Copy Markdown

Fixes COLLECTIONS-899, following the discussion there.

What

As discussed in the issue, CaseInsensitiveMap deliberately converts each character with Character.toLowerCase(Character.toUpperCase(c)) since COLLECTIONS-294, which avoids the locale problems of String.toLowerCase(). Two things did not match that, though:

  • The class Javadoc says that keys are "converted to all lowercase", which suggests 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.
  • As Bernd noted in the issue, convertKey() works on chars, 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 with Character.toLowerCase(Character.toUpperCase(int)) and appends it with StringBuilder.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.
  • The class Javadoc and the Javadoc of convertKey() now say what the conversion does: the simple, one-to-one Unicode case mappings, independently of the locale, which is not the same as String.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:

  • testSupplementaryCharacters puts 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.
  • testUncasedSupplementaryCharactersAndUnpairedSurrogates checks that an emoji and unpaired surrogates are kept as they are.
  • testConversionIsIdempotent checks, 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+). CaseInsensitiveMapTest passes on Java 8 and 17, and the default Maven goal passes locally on Java 17.

Before you push a pull request, review this list:

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute? Claude (Anthropic) helped analyse the issue and draft the change and tests; I reviewed them against the code and ran the build.
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

…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).
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.

1 participant