-
Notifications
You must be signed in to change notification settings - Fork 2
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
adding addition methods to identity service client #526
base: main
Are you sure you want to change the base?
Conversation
@@ -2,11 +2,17 @@ | |||
|
|||
import com.fasterxml.jackson.annotation.JsonProperty; | |||
import java.net.URI; | |||
import java.util.List; | |||
import java.util.UUID; | |||
import no.unit.nva.commons.json.JsonSerializable; | |||
|
|||
public record GetCustomerResponse(@JsonProperty("id") URI id, UUID identifier, String name, String displayName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename this class? "GetCustomerResponse" has unfortunate connotations. CustomerDto or Customer?
.uri(customerId); | ||
return attempt(getHttpResponseCallable(request)) | ||
.map(this::validateResponse) | ||
.map(r -> mapResponse(GetCustomerResponse.class, r)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r?
var request = HttpRequest.newBuilder() | ||
.GET() | ||
.uri(customerId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be extracted, and add relevant headers (e.g. Accept, possibly Accept-Encoding) since there are some defaults that Java inserts (actually, check these by adding a breakpoint and get back to me)
var request = HttpRequest.newBuilder() | ||
.GET() | ||
.uri(constructListCustomerUri()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the extracted method
var request = HttpRequest.newBuilder() | ||
.GET() | ||
.uri(customerId) | ||
.build(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extract
Adding methods for fetching customer by id and listing all customers to IdentityServiceClient, so there is noe need to implement "complex configured" http client each time we need to make authenticated request to identity-service.