Feature #2536 : "READ_ENUM_KEYS_USING_INDEX" to match "WRITE_ENUM_KEYS_USING_INDEX" #3764

JooHyukKim · 2023-01-29T16:27:31Z

Description

Add enum features to deserialize using enum index
Matches WRITE_ENUM_KEYS_USING_INDEX symmetrically, (test case included)

Related Issue : Need a Feature like "READ_ENUM_KEYS_USING_INDEX" #2536

Let me know for anything, cheers

src/main/java/com/fasterxml/jackson/databind/deser/std/StdKeyDeserializer.java

yawkat · 2023-01-30T11:43:23Z

src/main/java/com/fasterxml/jackson/databind/deser/std/StdKeyDeserializer.java

@@ -417,6 +431,18 @@ private EnumResolver _getToStringResolver(DeserializationContext ctxt)
            }
            return res;
        }
+
+       private EnumResolver _getIndexResolver(DeserializationContext ctxt) {
+          EnumResolver res = _byIndexResolver;


this form of double-checked locking is not thread-safe, see https://shipilev.net/blog/2014/safe-public-construction/

_byIndexResolver needs to be volatile

src/main/java/com/fasterxml/jackson/databind/util/EnumResolver.java

src/test/java/com/fasterxml/jackson/databind/deser/enums/EnumDeserializationTest.java

JooHyukKim · 2023-01-30T12:26:48Z

Thanks for the guidances guys! New version with all the points implmented/revised up

src/main/java/com/fasterxml/jackson/databind/deser/std/StdKeyDeserializer.java

pjfanning · 2023-01-30T13:42:18Z

src/main/java/com/fasterxml/jackson/databind/deser/std/StdKeyDeserializer.java

+                    }
+                }
+            }
+            return _byNameResolver;


should be _byIndexResolver

src/main/java/com/fasterxml/jackson/databind/deser/std/StdKeyDeserializer.java

pjfanning

lgtm but will need @cowtowncoder's approval to merge

JooHyukKim · 2023-02-01T04:38:23Z

@cowtowncoder do you have any other opinion on this PR?

cowtowncoder · 2023-02-01T19:57:05Z

@cowtowncoder do you have any other opinion on this PR?

Unfortunately I haven't had time to look into it yet -- there's a big backlog of items. But I hope to have a look soon. Thank you for contributing this!

cowtowncoder · 2023-02-01T19:59:55Z

Ok, one quick note: I do not want to add a new DeserializationFeature, as those should ideally be more general, not datatype-specific: I know a few already exists but I hope they eventually get moved out (for 3.0).

Instead I would like it added using EnumFeature.READ_ENUM_KEYS_USING_INDEX; similar to how new options have been added using JsonNodeFeature.
That should be about as easy to use and offers more extensibility (number of DeserializationFeature entries is limited to 31, f.ex).

cowtowncoder · 2023-02-01T20:01:02Z

src/main/java/com/fasterxml/jackson/databind/util/EnumResolver.java

@@ -126,7 +126,27 @@ public static EnumResolver constructUsingToString(DeserializationConfig config,
                config.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS));
    }

-    /**
+


Need to add Javadoc that includes @since 2.15 to indicate when method was added.

cowtowncoder · 2023-02-01T20:02:57Z

Aside from DeserializationFeature -> EnumFeature, and need for minor addition of Javadocs (to indicate version public things were added in), looks pretty good! Good job.
Will be happy to merge after changes.

JooHyukKim · 2023-02-01T22:18:35Z

Crossed out all above because @cowtowncoder explained about enabling EnumFeature support more in the futurue.

> Ok, one quick note: I do not want to add a new `DeserializationFeature`, as those should ideally be more general, not datatype-specific: I know a few already exists but I hope they eventually get moved out (for 3.0).
I see, these feature are to be moved out. So do we not need this symmetry with I am curious about your opinion on this.

Instead I would like it added using EnumFeature.READ_ENUM_KEYS_USING_INDEX; similar to how new options have been added using JsonNodeFeature. That should be about as easy to use and offers more extensibility (number of DeserializationFeature entries is limited to 31, f.ex).

Just to be clear, are we keeping both, or just EnumFeature?
I am confused if I should remove first DeserializationFeature. READ_ENUM_KEYS_USING_INDEX before implementing just adding aEnumFeature ?

…ithub.com/JooHyukKim/jackson-databind into FasterXML#2536-READ_ENUM_KEYS_USING_INDEX

cowtowncoder · 2023-02-06T02:18:34Z

Crossed out all above because @cowtowncoder explained about enabling EnumFeature support more in the futurue.

Right: this is about transitioning enum-related features (over time) to separate EnumFeature.
Existence of old settings is definitely bit confusing, thank you for picking this up here.

cowtowncoder · 2023-02-06T02:45:46Z

Thank you @JooHyukKim for contributing this -- I finally had time to merge it, and it will go in 2.15.0 release.

JooHyukKim added 6 commits January 29, 2023 22:54

Update DeserializationFeature.java

9dee90e

Feature : running using index keys

092cf5f

Test : add test cases

4f31887

Fix : Typo name to ENUMS to ENUm

ccbb813

Fix doc

0d2028d

Fix tests

58691c6

JooHyukKim mentioned this pull request Jan 30, 2023

Add EnumFeature.READ_ENUM_KEYS_USING_INDEX to work with existing "WRITE_ENUM_KEYS_USING_INDEX" #2536

Closed

yawkat reviewed Jan 30, 2023

View reviewed changes

src/main/java/com/fasterxml/jackson/databind/deser/std/StdKeyDeserializer.java Outdated Show resolved Hide resolved

yawkat reviewed Jan 30, 2023

View reviewed changes

src/main/java/com/fasterxml/jackson/databind/util/EnumResolver.java Outdated Show resolved Hide resolved

pjfanning reviewed Jan 30, 2023

View reviewed changes

src/test/java/com/fasterxml/jackson/databind/deser/enums/EnumDeserializationTest.java Outdated Show resolved Hide resolved

JooHyukKim added 4 commits January 30, 2023 21:06

Refactor : fix indents

17093f8

Refactor : fix indent

9d43e97

Refactor : fix indent

be2a69d

Fix : double-checked locking that is not thread-safe

a1d5983

Fix : dobule check locking

ce580eb

JooHyukKim mentioned this pull request Jan 30, 2023

Fix : Not thread-safe singleton factory methods on EnumResolver and StdKeyDeserializer class #3768

Merged

pjfanning reviewed Jan 30, 2023

View reviewed changes

src/main/java/com/fasterxml/jackson/databind/deser/std/StdKeyDeserializer.java Show resolved Hide resolved

pjfanning reviewed Jan 30, 2023

View reviewed changes

src/main/java/com/fasterxml/jackson/databind/deser/std/StdKeyDeserializer.java Show resolved Hide resolved

Update StdKeyDeserializer.java

5fc1c97

pjfanning reviewed Jan 30, 2023

View reviewed changes

JooHyukKim added 2 commits January 30, 2023 22:42

Fix : wrong reference

4628e02

Update StdKeyDeserializer.java

c69c709

JooHyukKim requested review from pjfanning and yawkat and removed request for pjfanning and yawkat January 30, 2023 15:24

JooHyukKim requested a review from yawkat January 30, 2023 15:24

pjfanning requested changes Jan 30, 2023

View reviewed changes

src/main/java/com/fasterxml/jackson/databind/deser/std/StdKeyDeserializer.java Outdated Show resolved Hide resolved

Fix : unthread-safe initilization like FasterXML#3768

03815dc

JooHyukKim requested review from pjfanning and removed request for yawkat January 31, 2023 03:04

pjfanning approved these changes Jan 31, 2023

View reviewed changes

cowtowncoder reviewed Feb 1, 2023

View reviewed changes

JooHyukKim added 5 commits February 3, 2023 21:56

replace DeserializationFeature.READ_ENUM_KEYS_USING_INDEX to EnumFeature

e7d164e

Fix tests

a227fdb

Merge branch '2.15' into FasterXML#2536-READ_ENUM_KEYS_USING_INDEX

81b2041

Add @SInCE javadoc

07305c6

Merge branch 'FasterXML#2536-READ_ENUM_KEYS_USING_INDEX' of https://g…

981486e

…ithub.com/JooHyukKim/jackson-databind into FasterXML#2536-READ_ENUM_KEYS_USING_INDEX

JooHyukKim requested review from cowtowncoder and yawkat and removed request for cowtowncoder and yawkat February 3, 2023 13:03

cowtowncoder approved these changes Feb 6, 2023

View reviewed changes

cowtowncoder merged commit d75496b into FasterXML:2.15 Feb 6, 2023

cowtowncoder added this to the 2.15.0 milestone Feb 6, 2023

JooHyukKim deleted the #2536-READ_ENUM_KEYS_USING_INDEX branch May 22, 2023 13:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature #2536 : "READ_ENUM_KEYS_USING_INDEX" to match "WRITE_ENUM_KEYS_USING_INDEX" #3764

Feature #2536 : "READ_ENUM_KEYS_USING_INDEX" to match "WRITE_ENUM_KEYS_USING_INDEX" #3764

JooHyukKim commented Jan 29, 2023 •

edited

yawkat Jan 30, 2023

JooHyukKim commented Jan 30, 2023 •

edited

pjfanning Jan 30, 2023

pjfanning left a comment

JooHyukKim commented Feb 1, 2023

cowtowncoder commented Feb 1, 2023

cowtowncoder commented Feb 1, 2023

cowtowncoder Feb 1, 2023

cowtowncoder commented Feb 1, 2023

JooHyukKim commented Feb 1, 2023 •

edited

cowtowncoder commented Feb 6, 2023

cowtowncoder commented Feb 6, 2023

Feature #2536 : "READ_ENUM_KEYS_USING_INDEX" to match "WRITE_ENUM_KEYS_USING_INDEX" #3764

Feature #2536 : "READ_ENUM_KEYS_USING_INDEX" to match "WRITE_ENUM_KEYS_USING_INDEX" #3764

Conversation

JooHyukKim commented Jan 29, 2023 • edited

Description

yawkat Jan 30, 2023

Choose a reason for hiding this comment

JooHyukKim commented Jan 30, 2023 • edited

pjfanning Jan 30, 2023

Choose a reason for hiding this comment

pjfanning left a comment

Choose a reason for hiding this comment

JooHyukKim commented Feb 1, 2023

cowtowncoder commented Feb 1, 2023

cowtowncoder commented Feb 1, 2023

cowtowncoder Feb 1, 2023

Choose a reason for hiding this comment

cowtowncoder commented Feb 1, 2023

JooHyukKim commented Feb 1, 2023 • edited

cowtowncoder commented Feb 6, 2023

cowtowncoder commented Feb 6, 2023

JooHyukKim commented Jan 29, 2023 •

edited

JooHyukKim commented Jan 30, 2023 •

edited

JooHyukKim commented Feb 1, 2023 •

edited