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

Issue/llainez update javax deps #2

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

LuisLainez
Copy link
Owner

@LuisLainez LuisLainez commented Apr 24, 2024

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • WHOSUSING.md
  • Other (please describe):

NOTE: Please remember to run ./gradlew spotlessApply to fix any format violations.

Changes in this PR

Describe the new behavior from this PR, and why it's needed
Issue #

Alternatives considered

Describe alternative implementation you have considered


Current Status

Struggling with the parsing into a ClientResponse - it keeps trying to deserialize the element as a ClientResponse, but that’s not right.

The hard code is getForEntity

ERROR com.netflix.conductor.client.http.ClientBase [] - Unable to invoke Conductor API with uri: http://localhost:8080/api/workflow/d045c33f-9728-4702-b0b9-bf868c01c75f?includeTasks=true, runtime exception occurred
jakarta.ws.rs.client.ResponseProcessingException:
com.fasterxml.jackson.databind.exc.InvalidDefinitionException:
Cannot construct instance of org.glassfish.jersey.client.ClientResponse
(no Creators, like default constructor, exist):
cannot deserialize from Object value (no delegate- or property-based Creator)

Journal

We deleted body from the delete request. This is because body is not really accepted in delete requests and will even be rejected sometimes.

We use ClientRequestFilter in the conductor-oss client as it’s for the client side: https://stackoverflow.com/questions/27578134/clientrequestfilter-vs-containerrequestfilter/27578248#27578248 - and register them via register https://docs.oracle.com/javaee/7/tutorial/jaxrs-client003.htm

We had to change jackson to

com.fasterxml.jackson.jakarta.rs:jackson-jakarta-rs-json-provider

as

com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider

uses the javax.ws. namespace instead of Jakarta and new Jersey doesnt like that

As we as upgrading

implementation "com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.17.0"

to the same version of Jackson.

webResourceBuilder no longer allows null as responseType, so I had to get the null as property type off the calls

webResourceBuilder.post(Entity.json(request), null) → webResourceBuilder.post(Entity.json(request))

I had to add

implementation "jakarta.ws.rs:jakarta.ws.rs-api:${revJAXRS}"
implementation 'jakarta.platform:jakarta.jakartaee-api:10.0.0' //Needed to jakarta.ws.rs-api

to common build.gradle because it is where the object mapper to read the tasks as ClientResponse, via Jersey, and it was failing ‘cause it uses MultiMap that is an abstract class and can’t initialize it.

I had to change the calls to ‘put’ with null requests as PUT does not accept a null entity.

WIP

Cleanup the JsonModule that introduces the changes for MultiMap + clean dependencies

Clean up the UniformInterfaceException that is not from Jersey

https://stackoverflow.com/questions/32042944/upgrading-from-jersey-client-1-x-to-jersey-client-2-x

https://eclipse-ee4j.github.io/jersey.github.io/documentation/latest/client.html

https://gist.github.com/aphexmunky/11399575

https://stackoverflow.com/questions/67013351/convert-jersey-1-x-client-to-jersey-2-x-client

https://stackoverflow.com/questions/53843533/clienthandlerexception-jersey

https://stackoverflow.com/questions/42468133/registering-jacksonjsonprovider-with-objectmapper-javatimemodule-to-jersey-2-c

https://stackoverflow.com/questions/32042944/upgrading-from-jersey-client-1-x-to-jersey-client-2-x

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

Successfully merging this pull request may close these issues.

1 participant