chore(pom.xml): Update json-path to 2.0.0

[escardin]

• Update json-path to 2.0.0
• Reintroduce minidev dependency (json-path defaults depends on it if alternative defaults are not set before building a configuration)
• Delete now unecessary JsonPathJacksonProvider

[meyerdan]

Thank you for this pull request.

Why is minidev introduced? I did not understand that completely.

Daniel

[escardin]

It's a bit convoluted (and kind of a bug in jsonpath IMO), but I was not comfortable enough with your code working around it. I personally would not want it included, as I already have jackson and gson dependencies in my project.

In the PR, you call JsonPath's configuration builder:
new ConfigurationBuilder()
.jsonProvider(new JacksonJsonProvider(objectMapper))
.mappingProvider(new JacksonMappingProvider(objectMapper))
.build());

The call to build() internally calls getEffectiveDefaults().

getEffectiveDefaults() if you have not supplied a default configuration, uses DefaultsImpl.INSTANCE which calls out to the minidev implementation.

My original solution was to set the defaults myself, but it would make the constructor chain rather awkward, and I would rather not set defaults if some other library user wants minidev.

I also can't call the Configuration constructor directly, as it is private.

[ThorbenLindhauer]

I had similar issues when I considered upgrading to 1.2.0 half a year ago. I created a bug ticket in the json-path project (https://code.google.com/p/json-path/issues/detail?id=68) but didn't have the time to provide a pull request for it. In my opinion, upgrading json-path is not worth adding a new dependency, so we should either fix this in json-path first or simply not upgrade. Is there a specific use case or bug you'd like to upgrade json-path for?

[escardin]

I've filed a bug with a proposed fix in json path. json-path/JsonPath#121

It's not directly a problem for me at this time, but I make use of other libraries when testing (restassured) which depend on 2.0.0 and it could be a classpath conflict at that time. So I would really like it fixed, but I am comfortable waiting until it causes me an actual issue.

I don't like having old unsupported libraries in my project either, it always ends up being a problem.

[ThorbenLindhauer]

Thanks for the effort. I agree that using a more recent version is generally a good thing.

[ThorbenLindhauer]

I have seen they have resolved the issue with json-smart in json-path. Could you please update your pull request once this change makes it into a release?

[ThorbenLindhauer]

Hi @escardin,

Are you still interested in getting this done? I just tried applying your patch with json-path version 2.1.0 that contains the fix you submitted to json-path to exclude the json-smart dependency. However, it doesn't seem to be sufficient since there are more points in which json-path instantiates new Configuration objects that try to load json-smart classes (one such point being https://github.com/jayway/JsonPath/blob/json-path-2.1.0/json-path/src/main/java/com/jayway/jsonpath/internal/filter/ValueNode.java#L778 It hands over the Jackson Json provider but not the mapping provider so it tries to instantiate one using json-smart).

Are you willing to look into this and take this further? If you currently do not have the time/interest, I will close the pull request and we can re-open it any time.

Cheers,
Thorben

[escardin]

They may have been other changes between 2.0.0 and 2.1.0. I will look into it, but probably not very soon. Closing is fine.

[ThorbenLindhauer]

Sure, just reopen when you have the time. Thanks for your efforts.

[stefanzilske]

Hi folks,

any news on this? I would love to see camunda-spin using an up-to-date version of json-path, because it inteferes e.g. with SpEL #jsonPath, which requires a newer version.

Cheers,
Stefan

[ThorbenLindhauer]

Hi Stefan,

At Camunda, this is currently not scheduled. Feel free to continue the work that was already done and submit a separate pull request.

Cheers,
Thorben

[stefanzilske]

@ThorbenLindhauer

did I get it right, you don't want to have the minidev-dependency? Even with json-path 2.2.0 it seems to be required. We can of course try to fix this in json-path, but why not just include it in camunda-spin?

Cheers,
Stefan

[ThorbenLindhauer]

Hi Stefan,

Yes, we don't want minidev. The reason is that we have already Jackson in Spin and that Jackson can be used a replacement for minidev in json-path. If json-path does not work with minidev excluded, then we should suggest a fix there. The general reason is that we try to keep the list of dependencies that Camunda's core libraries require to a minimum. The less dependencies the less worries when running things in a shared environment like an application server.

Cheers,
Thorben

[stefanzilske]

@ThorbenLindhauer

Sure sure... I will have to submit a PR to json-path first, to copy over also the mapping provider (what you actually already proposed).

On the other hand, reading through issues and pull requests for json-path, I get the impression that they do not want to make mindev optional. They stated that several times.

I don't think we should make/have made json-smart optional.

So even if we fix the place you mentioned in ValueNode (which would indeed make minidev obsolete for spin, I fixed and tested it locally), there is no guarantee that it won't become mandatory again with future releases.

So maybe we should look for a replacement of json-path or accept the minidev dependency?

[ThorbenLindhauer]

Thanks for your efforts. Could you please link to the comment you quoted?

[stefanzilske]

Sure, I found several comments, where this is mentioned:

• Feature Request: Make dependencies on the different providers json-smart, jackson, gson optional json-path/JsonPath#101
• Predicate testing forces dependency on Minidev json-smart json-path/JsonPath#274
• use json-smart to parse the query json-path/JsonPath#314

[ThorbenLindhauer]

I don't find the comments really clear, so I guess the best way is to ask, see json-path/JsonPath#349

[ThorbenLindhauer]

Hi @stefanzilske,

My issue has been finally responded to indicating that json-path is a required dependency. I'll discuss this internally and then get back to you. Please poke me should I forget :)

Cheers,
Thorben

[ThorbenLindhauer]

Quick update: Having a new dependency is generally fine for us in this case unless something breaks. @stefanzilske: Would you be interested in raising a new pull request that updates json-path to the latest version?

[stefanzilske]

@ThorbenLindhauer

sorry for the late reply. I think I will provide the PR today (as I had already prepared it).

Cheers,
Stefan�