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

Changing JsonPathProvider to Jackson breaks the JsonPathLinkDiscoverer #1980

Closed
BeniaminK opened this issue May 17, 2023 · 3 comments
Closed
Assignees
Labels
in: core Core parts of the project type: bug
Milestone

Comments

@BeniaminK
Copy link

Changing JsonPath provider to JacksonProvider breaks the createLinksFrom in JsonPathLinkDiscoverer

By setting the Configuration of the JsonPath to JacksonJsonProvider - the createLinksFrom breaks it's usage (as it relies only on JsonSmartJsonPRovider)

Configuration.setDefaults(new Configuration.Defaults() {

    private final JsonProvider jsonProvider = new JacksonJsonProvider();
    private final MappingProvider mappingProvider = new JacksonMappingProvider();
      
    @Override
    public JsonProvider jsonProvider() {
        return jsonProvider;
    }

    @Override
    public MappingProvider mappingProvider() {
        return mappingProvider;
    }
    
    @Override
    public Set<Option> options() {
        return EnumSet.noneOf(Option.class);
    }
});

breaks almost all tests in the HalLinkDiscovererUnitTest.

However adding:

if (List.class.isInstance(parseResult)) {

	List<?> list = (List<?>) parseResult;

	return list.stream()
			.flatMap(it -> List.class.isInstance(it) ? ((List<?>) it).stream() : Stream.of(it))
			.map(it -> extractLink(it, rel))
			.collect(Links.collector());
}

in the JsonPathLinkDiscoverer file fixes those Tests (however still few other tests are broken).

I think the implementation in Spring shouldn't so closely rely on using net.minidev.json packages

@odrotbohm
Copy link
Member

odrotbohm commented Jun 29, 2023

Would you mind stepping back and elaborate what you're trying to achieve, exactly? Ideally, in the form of a test case?

If customizing the configuration changes the way JsonPath works so fundamentally, is that maybe something worth bringing up with the JsonPath team? Just asking because we're not actively relying on JsonSmartJsonProvider but consistency in what's returned from JsonPath.read(…). If that significantly changes depending on the JsonProvider implementation, this is definitely something to discuss over there, as one cannot reliably write code against JsonPath then. How would we know, which cases we have to cover?

@odrotbohm
Copy link
Member

I've investigated a little more, and it looks like we can indeed simply switch to List over JSONArray. It seems like the latter has always implemented List, so I don't remember exactly why we coded against the concrete class in the first place. Good catch, though!

@odrotbohm odrotbohm added type: bug in: core Core parts of the project and removed process: waiting for feedback labels Jun 29, 2023
@odrotbohm odrotbohm added this to the 2.2.0-M1 milestone Jun 29, 2023
@odrotbohm odrotbohm changed the title Changing JsonPath provider to JacksonProvider breaks the JsonPathLinkDiscoverer Changing JsonPathProvider to Jackson breaks the JsonPathLinkDiscoverer Jun 29, 2023
odrotbohm added a commit that referenced this issue Jun 29, 2023
So far, the handling of links detected via a JSON Path expression has assumed that the JsonProvider would always return JSONArray objects for collections. However, the JacksonJsonProvider for example, returns plain List instances. As JSONArray implements List, we now only refer to the latter for maximum compatibility with different JsonProvider implementations.
odrotbohm added a commit that referenced this issue Jun 29, 2023
So far, the handling of links detected via a JSON Path expression has assumed that the JsonProvider would always return JSONArray objects for collections. However, the JacksonJsonProvider for example, returns plain List instances. As JSONArray implements List, we now only refer to the latter for maximum compatibility with different JsonProvider implementations.

Original ticket: #1980
odrotbohm added a commit that referenced this issue Jun 29, 2023
So far, the handling of links detected via a JSON Path expression has assumed that the JsonProvider would always return JSONArray objects for collections. However, the JacksonJsonProvider for example, returns plain List instances. As JSONArray implements List, we now only refer to the latter for maximum compatibility with different JsonProvider implementations.

Original ticket: #1980
odrotbohm added a commit that referenced this issue Jun 29, 2023
So far, the handling of links detected via a JSON Path expression has assumed that the JsonProvider would always return JSONArray objects for collections. However, the JacksonJsonProvider for example, returns plain List instances. As JSONArray implements List, we now only refer to the latter for maximum compatibility with different JsonProvider implementations.

Original ticket: #1980
@odrotbohm
Copy link
Member

That's fixed and back-ported as far as 1.5.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Core parts of the project type: bug
Projects
None yet
Development

No branches or pull requests

2 participants