Skip to content
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

Guice Annotation Introspection doesn't observer JacksonInject.useInput values #134

Closed
josephlbarnett opened this issue May 3, 2021 · 4 comments

Comments

@josephlbarnett
Copy link
Contributor

When trying to do something similar to FasterXML/jackson-databind#962 while using the jackson guice module to provide injectable values from the guice bindings, setting the useInput=OptBoolean.FALSE property is ineffective. It appears that https://github.com/FasterXML/jackson-modules-base/blob/2.12/guice/src/main/java/com/fasterxml/jackson/module/guice/GuiceAnnotationIntrospector.java#L22 always constructs a JacksonInject.Value with useInput=null, so there is no way to tell jackson the injected value should be "inject-only".

josephlbarnett added a commit to josephlbarnett/jackson-modules-base that referenced this issue May 5, 2021
Constructs the JacksonInject.Value instance with the useInput from
the annotation instead of always setting it to null/DEFAULT.

attempts to fix FasterXML#134
@josephlbarnett
Copy link
Contributor Author

Have attempted to look at this here: 2.12...josephlbarnett:guice-annotation-use-input

This allows me to avoid the injection failure from FasterXML/jackson-databind#962 by setting useInput=OptBoolean.FALSE, but as seen in the attached test that fails, it appears that the injected value is always used even in the default/TRUE cases? For my specific use case that's ok, but seems unexpected / like I'm missing something about how this should work?

@cowtowncoder
Copy link
Member

cowtowncoder commented May 5, 2021

If suggesting this change, it probably would need to be a configurable setting just to make sure.

Ideally AnnotationIntrospector here should not really have to do this processing; it should usually be up to AnnotationIntrospectorPair.java to merge possible values.

Come to think it now, it looks like implementation there is the problem:

    public JacksonInject.Value findInjectableValue(AnnotatedMember m) {
        JacksonInject.Value r = _primary.findInjectableValue(m);
        return (r == null) ? _secondary.findInjectableValue(m) : r;
    }

since it probably should be doing this merging...

Would you be interested in trying a PR for jackson-databind (branch 2.12) for that?
(file is src/main/java/com/fasterxml/jackson/databind/introspect/AnnotationIntrospectorPair.java in jackson-databind)

@josephlbarnett
Copy link
Contributor Author

ah the injected value issue appears to be similar if not the same as FasterXML/jackson-databind#2678

I can see if a change to AnnotationIntrospectorPair to merge id/useInputs from primary/secondary would help with the FasterXML/jackson-databind#962 part of the problem -- should it be something along the lines of "use the primary value, but if useInput is null/default, merge primary id with secondary useInput"?

@cowtowncoder
Copy link
Member

@josephlbarnett yes, merging should only "fill in the blanks", and OptBoolean is designed to allow "not defined" (instead of boolean property).

josephlbarnett added a commit to josephlbarnett/jackson-databind that referenced this issue May 6, 2021
As many AnnotationIntrospector implementations use default values
for useInput, allow the secondary introspector's useInput value to
combine with the primary's id to prevent losing the useInput value.

Fixes a special case of FasterXML#962 seen by the GuiceAnnotationInspector
in FasterXML/jackson-modules-base#134
josephlbarnett added a commit to josephlbarnett/jackson-databind that referenced this issue May 6, 2021
As many AnnotationIntrospector implementations use default values
for useInput, allow the secondary introspector's useInput value to
combine with the primary's id to prevent losing the useInput value.

Fixes a special case of FasterXML#962 seen by the GuiceAnnotationInspector
in FasterXML/jackson-modules-base#134
cowtowncoder pushed a commit to FasterXML/jackson-databind that referenced this issue May 7, 2021
…3146)

As many AnnotationIntrospector implementations use default values
for useInput, allow the secondary introspector's useInput value to
combine with the primary's id to prevent losing the useInput value.

Fixes a special case of #962 seen by the GuiceAnnotationInspector
in FasterXML/jackson-modules-base#134
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

2 participants