External type property (`@JsonTypeInfo(include = As.EXTERNAL_PROPERTY`)) deserialized even if `visible = false`

[rocketraman]

versions: Jackson 2.8.1, Jackson-module-kotlin 2.8.1

Since I am running into issue #1328 with external property deserialization, I am falling back to a secondary non-enum field which is used by the mapper instead. However, I don't care about the value of this secondary field, so I set visible to false. However Jackson deserializes the external type value despite visible=false. In version 2.6.7 visible=false in the exact same scenario had the correct behavior.

Reproduction recipe:
https://github.com/rocketraman/jackson-issue-enum-polymorphism/blob/master/src/main/kotlin/visibility/Reproduction-VisibleFalse.kt

The output of the program should be:

Invite(kind=CONTACT, kindForMapper=null, to=InviteToContact(name=Foo))

but it is:

Invite(kind=CONTACT, kindForMapper=CONTACT, to=InviteToContact(name=Foo))

[apatrida]

I added a unit test for this to the Jackson-Kotlin module. It broke somewhere in the 2.8.x release cycle.

• When running with databind 2.8.0.rc2 the unit test passes.
• When running with databind 2.8.0, 2.8.1 and 2.8.2, it fails

I do not change the Kotlin module verison, only the databind version. The Unit test was added to Kotlin-Jackson module as GithubDatabind1329.kt and is marked ignored there for now.

@cowtowncoder, ideas?

[cowtowncoder]

@apatrida unfortunately no idea; can not remember any changes to handling of @JsonTypeInfo. If possible it would be great to have Java-only reproduction since it sounds like this would be databind problem, and not something to do with Kotlin side of things.

[rocketraman]

@cowtowncoder (cc: @apatrida ) Here is a java-only reproduction of this issue:

https://github.com/rocketraman/jackson-issue-enum-polymorphism/blob/master/src/main/java/visibility/java/ReproductionVisibleFalse.java

package visibility.java;

import com.fasterxml.jackson.databind.ObjectMapper;

import java.io.IOException;

class ReproductionVisibleFalse {
  public static void main(String[] args) throws IOException {
    ObjectMapper mapper = new ObjectMapper();
    Invite invite = mapper.readValue("{ \"kind\": \"CONTACT\", \"kindForMapper\": \"CONTACT\", \"to\": { \"name\": \"Foo\" } }", Invite.class);

    // this should be Invite(kind=CONTACT, kindForMapper=null, to=InviteToContact(name=Foo))
    // but it will be Invite(kind=CONTACT, kindForMapper=CONTACT, to=InviteToContact(name=Foo))
    System.out.println(invite);
  }
}

[cowtowncoder]

Tried a fix with Claude but didn't quite get there: may try again in future. Summary of initial effort as follows:

---

Session Summary: Fix for Jackson Issue #1329

Issue

Fix GitHub issue #1329: "External property deserialized even if visible = false"

When @JsonTypeInfo(include=EXTERNAL_PROPERTY, visible=false) is used, the external type property should NOT be set on the parent bean — it should only be used for type resolution. This was a regression introduced in 2.8.0+.

What Was Done

New files created

• src/test/java/tools/jackson/databind/jsontype/ext/ExternalTypeIdWithVisible1329Test.java
  • Two tests: testExternalTypeIdNotVisibleInTypeInfo() and testExternalTypeIdWithVisibleFalse()
  • Moved/rewritten from old tofix/ExternalTypeIdWithVisible1329Test.java (which used @JacksonTestFailureExpected)

Files deleted

• src/test/java/tools/jackson/databind/tofix/ExternalTypeIdWithVisible1329Test.java

Files modified

1. src/main/java/tools/jackson/databind/jsontype/TypeDeserializer.java

   • Added isTypeIdVisible() default method returning false

2. src/main/java/tools/jackson/databind/jsontype/impl/TypeDeserializerBase.java

   • Overrode isTypeIdVisible() to return _typeIdVisible

3. src/main/java/tools/jackson/databind/deser/impl/ExternalTypeHandler.java

   • Added isTypeIdVisible() to ExtTypedProperty inner class
   • Added isTypePropertyVisible(String propName) public method to ExternalTypeHandler

4. src/main/java/tools/jackson/databind/deser/bean/BeanDeserializer.java

   • Modified _deserializeWithExternalTypeId(): after handleTypePropertyValue() returns true, skip prop.deserializeAndSet() if !ext.isTypePropertyVisible(propName)
   • Modified deserializeUsingPropertyBasedWithExternalTypeId() similarly

5. src/test/java/tools/jackson/databind/jsontype/ext/ExternalTypeIdTest.java

   • testIssue831(): changed assertEquals("dog", result.petType) → assertNull(result.petType)
   • testMultipleValuesSingleExtId291(): changed assertEquals("1", c.type) → assertNull(c.type)
   • testBigDecimal965(): changed assertEquals(w.typeEnum, w2.typeEnum) → assertNull(w2.typeEnum)
   • testBigDecimal965StringBased(): same as above

Current Failing Tests (5 failures)

The fix is too broad. Running ./mvnw test -Dtest="tools.jackson.databind.jsontype.**" shows:

ExternalTypeIdWithCreatorTest.testExternalIdWithAnySetter3045:303
  expected: <track> but was: <null>

TestSubtypesExternalPropertyMissingProperty.testDeserializationEmpty:155
  expected: <orange> but was: <null>

TestSubtypesExternalPropertyMissingProperty.testDeserializationMissing:169
  expected: <orange> but was: <null>

TestSubtypesExternalPropertyMissingProperty.testDeserializationNull:141
  expected: <orange> but was: <null>

TestSubtypesExternalPropertyMissingProperty.testDeserializationPresent:127
  expected: <orange> but was: <null>

Root Cause of Failures

Both TestSubtypesExternalPropertyMissingProperty.Box and ExternalTypeIdWithCreatorTest.MyJson3045 have a plain String type field that coincides with the external type property name "type". The current fix suppresses setting this field on the bean when visible=false (which is the default). This is correct per visible=false semantics, but breaks these tests which were written expecting the "buggy" behavior.

Decision Point

There are two options going forward:

Option A: Update the failing tests to expect null (correct behavior)

• TestSubtypesExternalPropertyMissingProperty: Change assertEquals(orangeBox.type, deserOrangeBox.type) (which compares "orange" to "orange") to assertNull(deserOrangeBox.type)
• ExternalTypeIdWithCreatorTest.testExternalIdWithAnySetter3045: Change assertEquals("track", result.type) to assertNull(result.type)
• This is the correct semantic fix — visible=false means don't set the property on the bean
• This is a breaking change for users who relied on the regression behavior

Option B: Revert the fix and find a more targeted approach

• Only apply the suppression when the property name matches an external type property AND the user EXPLICITLY set visible=false
• Problem: Cannot distinguish explicit visible=false from the default visible=false in the annotation

Recommendation

Option A is most likely correct. Both Box.type and MyJson3045.type are redundant fields that should be null when visible=false. These tests were written after the 2.8 regression and reflect wrong expected behavior. The fix is semantically correct — visible=false means "don't expose the type id to the deserializer." Updating the failing tests is the right approach.

Commands to Run Tests

# Run just the new test
./mvnw test -Dtest=ExternalTypeIdWithVisible1329Test

# Run all jsontype tests (shows 5 failures currently)
./mvnw test -Dtest="tools.jackson.databind.jsontype.**"

# Run specific failing tests
./mvnw test -Dtest=TestSubtypesExternalPropertyMissingProperty
./mvnw test -Dtest=ExternalTypeIdWithCreatorTest#testExternalIdWithAnySetter3045