Skip to content

Unwrapping lists does not work inside @JsonUnwrapped #762

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

Open
SimonCockx opened this issue May 21, 2025 · 1 comment
Open

Unwrapping lists does not work inside @JsonUnwrapped #762

SimonCockx opened this issue May 21, 2025 · 1 comment

Comments

@SimonCockx
Copy link
Contributor

@JacksonXmlElementWrapper(useWrapping = false) results in an exception being thrown when used inside a @JsonUnwrapped property:

java.lang.IllegalStateException: Internal error: should never get called
	at com.fasterxml.jackson.dataformat.xml.deser.WrapperHandlingDeserializer.newDelegatingInstance(WrapperHandlingDeserializer.java:64)

A small reproducer of what I'm trying to achieve is described below. Note that my example does not make sense in any real application without the context of my actual application, but it's a clear minimal example to show what is happening.

XML I'm deserializing:

<pojo>
  <value>a</value>
  <value>b</value>
</pojo>

Classes I'm deserializing to:

public class Pojo {
    private Nested nested;
    
    @JsonUnwrapped
    public Nested getNested() {
        return nested;
    }
    public void setNested(Nested nested) {
        this.nested = nested;
    }
}
public class Nested {
    private List<String> values;
    
    @JacksonXmlElementWrapper(useWrapping = false)
    @JacksonXmlProperty(localName = "value")
    public List<String> getValues() {
        return values;
    }
    public void setValues(List<String> values) {
        this.values = values;
    }
}

The following code then throws the aformentioned exception:

XmlMapper mapper = new XmlMapper();
Pojo actual = mapper.readValue(xml, Pojo.class);

Related: #676
The order of "unwrapping" here is the other way around.

@SimonCockx
Copy link
Contributor Author

I've implemented a solution, but it involves quite a bit of copy-pasting and overriding of Jackson internals. It might give an idea of what it takes to support this though, so I'll describe my solution below.

Step 1: make sure that the IllegalStateException is not thrown anymore.

Inside WrapperHandlingDeserializer, the following method exists:

@Override
protected JsonDeserializer<?> newDelegatingInstance(JsonDeserializer<?> newDelegatee0) {
    // default not enough, as we may need to create a new wrapping deserializer
    // even if delegatee does not change
    throw new IllegalStateException("Internal error: should never get called");
}

I've implemented it like this:

return new WrapperHandlingDeserializer((BeanDeserializerBase) unwrapping, _namesToWrap);

The next problem you'll encounter is that WrapperHandlingDeserializer#_configureParser method is not doing what it is supposed to do. It requires its passed JsonParser to be an instance of ElementWrappable, which is the case for FromXmlParser, but when @JsonUnwrapped is involved, the actual parser is an instance of TokenBuffer.Parser. This token buffer parser is created in UnwrappedPropertyHandler#processUnwrapped:

public Object processUnwrapped(JsonParser originalParser, DeserializationContext ctxt,
        Object bean, TokenBuffer buffered)
    throws IOException
{
    for (int i = 0, len = _properties.size(); i < len; ++i) {
        SettableBeanProperty prop = _properties.get(i);
        JsonParser p = buffered.asParser(originalParser.streamReadConstraints()); // SEE THIS LINE HERE
        p.nextToken();
        prop.deserializeAndSet(p, ctxt, bean);
    }
    return bean;
}

In order to support list unwrapping, we'll need that parser to be an instance of ElementWrappable. The following two steps achieve that.

Step 2: Implement a custom FromXmlTokenBuffer and wire it in.

The only difference with the TokenBuffer class is that it overrides the asParser(...) methods to return a custom Parser that implements ElementWrappable instead. (see step 3)

public class FromXmlTokenBuffer extends TokenBuffer {
    public FromXmlTokenBuffer(JsonParser p, DeserializationContext ctxt) {
        super(p, ctxt);
    }

    @Override
    public JsonParser asParser(ObjectCodec codec)
    {
        return new Parser(_first, codec, _hasNativeTypeIds, _hasNativeObjectIds, _parentContext, _streamReadConstraints);
    }

    @Override
    public JsonParser asParser(StreamReadConstraints streamReadConstraints)
    {
        return new Parser(_first, _objectCodec, _hasNativeTypeIds, _hasNativeObjectIds, _parentContext, streamReadConstraints);
    }

    @Override
    public JsonParser asParser(JsonParser src)
    {
        Parser p = new Parser(_first, src.getCodec(), _hasNativeTypeIds, _hasNativeObjectIds,
                _parentContext, src.streamReadConstraints());
        p.setLocation(src.currentTokenLocation());
        return p;
    }
}

To make sure the XML mapper uses this token buffer, we need to modify XmlDeserializationContext#bufferForInputBuffering:

@Override
public FromXmlTokenBuffer bufferForInputBuffering(JsonParser p) {
    return new FromXmlTokenBuffer(p, this);
}

Step 3: Implement a custom FromXmlTokenBuffer.Parser which implements ElementWrappable correctly.

At this point things get tricky. It seems that the current TokenBuffer.Parser does not implement any XML features as implemented in FromXmlParser at all. (e.g., auto-unwrapping of text values in a string collection) Just for the particular use case of this issue though, the following seems to suffice:

  1. Copy the TokenBuffer.Parser class. (it is final...)
  2. Replace the _parsingContext with a custom FromXmlTokenBufferReadContext, which extends TokenBufferReadContext and adds support for remembering which names to wrap:
public void setNamesToWrap(Set<String> namesToWrap) {
    _namesToWrap = namesToWrap;
}

public boolean shouldWrap(String localName) {
    return (_namesToWrap != null) && _namesToWrap.contains(localName);
}
  1. Implement support for element wrapping inside nextToken() and nextTextValue(). At this point I'm at a loss of what the best way of implementing this would be. For inspiration, here is my minimal solution just for this particular use case:
        @Override
        public JsonToken nextToken() throws IOException
        {
            // If we are closed, nothing more to do
            if (_closed || (_segment == null)) return null;
            
            if (wrapperState > 0) { // I've added a `wrapperState` property, which starts at 0.
                if (wrapperState == 1) {
                    wrapperState = 2;
                    _currToken = JsonToken.START_ARRAY;
                    return _currToken;
                } else if (wrapperState == 2) {
                    wrapperState = 3;
                    _currToken = JsonToken.FIELD_NAME;
                    return _currToken;
                } else if (wrapperState == 4) {
                    wrapperState = 0;
                    wrapperName = null;
                    _currToken = _segment.type(_segmentPtr);
                    return _currToken;
                }
            }

            // Ok, then: any more tokens?
            if (++_segmentPtr >= Segment.TOKENS_PER_SEGMENT) {
                _segmentPtr = 0;
                _segment = _segment.next();
                if (_segment == null) {
                    return null;
                }
            }
            JsonToken t = _segment.type(_segmentPtr);
            // Field name? Need to update context
            if (t == JsonToken.FIELD_NAME) {
                Object ob = _currentObject();
                String name = (ob instanceof String) ? ((String) ob) : ob.toString();
                if (wrapperState == 0) {
                    if (_parsingContext.shouldWrap(name)) { // We need to wrap!
                        wrapperName = name;
                        wrapperState = 1;
                    }
                } else if (wrapperState == 3) {
                    if (!_parsingContext.shouldWrap(name)) { // We need to stop wrapping.
                        wrapperState = 4;
                        _currToken = JsonToken.END_ARRAY;
                        return _currToken;
                    }
                }
                _parsingContext.setCurrentName(name);
            } else if (t == JsonToken.START_OBJECT) {
                _parsingContext = _parsingContext.createChildObjectContext();
            } else if (t == JsonToken.START_ARRAY) {
                _parsingContext = _parsingContext.createChildArrayContext();
            } else if (t == JsonToken.END_OBJECT
                    || t == JsonToken.END_ARRAY) {
                if (wrapperState == 3) { // We need to stop wrapping.
                    wrapperState = 4;
                    _currToken = JsonToken.END_ARRAY;
                    return _currToken;
                }
                // Closing JSON Object/Array? Close matching context
                _parsingContext = _parsingContext.parentOrCopy();
            } else {
                _parsingContext.updateForValue();
            }
            _currToken = t;
            return _currToken;
        }

I'm not saying that this is the right solution, but it does give an idea of what supporting this will involve.

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

No branches or pull requests

1 participant