2
\$\begingroup\$

The first request is sent asynchronously, then the id value is taken from the response to the first request and used in the second asynchronous request. And then both responses to request 1 and request 2 are combined. The example below works, but the code looks so clunky. Is there a better, shorter way to implement this? Maybe a separate class should be created for 'new Callback()' or what would improve the code? The process in short - everything happens asynchronously:

  1. Send POST request 1
  2. Use the id value from response 1 for request 2
  3. Send POST request 2
  4. Combine response 1 and response 2 to the final response of the REST controller

This body is sent to the REST controller endpoint "/combine" via POST method:

{ "userid": 1, "id": 2, "title": "3", "body": "4" }

@RestController
public class CombinationController {
  @Autowired CombinationService combinationService;

  @PostMapping("/combine")
  public CompletableFuture<CombinationBothResponses> combine(
      @RequestBody RequestBodyOne requestBodyOne) {
    CompletableFuture<CombinationBothResponses> resultCallback = new CompletableFuture<>();
    combinationService.sendTwoRequests(requestBodyOne, resultCallback);
    return resultCallback;
  }
}
@Service
public class CombinationService {
  private final Jsonb jsonb = JsonbBuilder.create();
  private final OkHttpClient client = new OkHttpClient();

  public void sendTwoRequests(
      RequestBodyOne requestBodyOne, CompletableFuture<CombinationBothResponses> resultCallback) {
    // Create request 1
    RequestBody requestBody =
        RequestBody.create(jsonb.toJson(requestBodyOne), MediaType.parse("application/json"));

    Request request =
        new Request.Builder()
            .url("https://jsonplaceholder.typicode.com/posts")
            .post(requestBody)
            .build();
    // 1. Send POST request 1
    client
        .newCall(request)
        .enqueue(
            new Callback() {
              @Override
              public void onResponse(@NotNull Call call, @NotNull Response response)
                  throws IOException {
                if (response.isSuccessful()) {
                  final CombinationBothResponses combinationBothResponses =
                    jsonb.fromJson(response.body().string(), CombinationBothResponses.class);
                  sendSecondPostRequest(combinationBothResponses, resultCallback);
                } else {
                  resultCallback.completeExceptionally(new RuntimeException());
                }
              }

              @Override
              public void onFailure(@NotNull Call call, @NotNull IOException e) {
                resultCallback.completeExceptionally(e);
              }
            });
  }

  private void sendSecondPostRequest(
      CombinationBothResponses combinationBothResponses,
      CompletableFuture<CombinationBothResponses> resultCallback) {
    // 2. Use one value from response 1 for request 2
    int valueToBeUsedInRequestBody2 = combinationBothResponses.getId();
    RequestBodyTwo requestBodyTwo = new RequestBodyTwo(valueToBeUsedInRequestBody2, "request 2");
    
    // Create request 2
    RequestBody requestBody =
        RequestBody.create(jsonb.toJson(requestBodyTwo), MediaType.parse("application/json"));

    Request request =
        new Request.Builder().url("https://reqbin.com/echo/post/json").post(requestBody).build();

    // 3. Send POST request 2
    client
        .newCall(request)
        .enqueue(
            new Callback() {
              @Override
              public void onResponse(@NotNull Call call, @NotNull Response response)
                  throws IOException {
                if (response.isSuccessful()) {
                  final CombinationBothResponses combinationBothResponses2 =
                    jsonb.fromJson(response.body().string(), CombinationBothResponses.class);
                // 4. Combine response 1 and response 2 to the final response of the REST controller
                combinationBothResponses.setSuccess(combinationBothResponses2.getSuccess());
                  resultCallback.complete(combinationBothResponses);
                } else {
                  resultCallback.completeExceptionally(new RuntimeException());
                }
              }

              @Override
              public void onFailure(@NotNull Call call, @NotNull IOException e) {
                resultCallback.completeExceptionally(e);
              }
            });
  }
}
@Data
@NoArgsConstructor
public class RequestBodyOne {
    private int userId;
    private int id;
    private String title;
    private String body;
}
@Data
@AllArgsConstructor
@NoArgsConstructor
public class RequestBodyTwo {
    private int id;
    private String key;
}
@Data
@NoArgsConstructor
public class CombinationBothResponses {
    private int userId;
    private int id;
    private String title;
    private String body;

    private String success;

}

Response to request 1:

{ "userId": 0, "id": 101, "title": "3", "body": "4" }

Response to request 2:

{"success":"true"}

Combined responses; response of REST controller:

{ "userId": 0, "id": 101, "title": "3", "body": "4", "success": "true" }

\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

Types

I find it mildly confusing to have a type named RequestOneBody be used not only for the first request's body but also for its response's body. The name seems to imply it'd be used for the former only. Additionally, since they seem to have different content (the request's id seems to be ignored, suggesting it can safely be left out, but the response will always have an id), it sort of feels like they should have different types

This is more noticeable with CombinationBothResponses, which is not only used for the combined data, but also for the response to request two on its own. I would've liked to see a see a separate class for the response for request two.

I also feel like the names could be more descriptive, but without knowing the actual purposes of the requests, I don't feel I can make any specific suggestions on that topic

Structure

I find the data flow to be a bit unintuitive. I feel it would be easier to follow if each request took its request body as input, and then instead of being void, returned its response body (likely wrapped in a CompletableFuture)

On a related note, there is a lot of duplicated code relating to putting together the RequestBodys, Requests and Callbacks.

We might be able to tackle both those issues with a method kind of like:

public <RequestType, ResponseType> CompletableFuture<? super ResponseType> makeRequest(
  String url,
  RequestType requestPayload,
  Class<ResponseType> resultType
) {
  CompletableFuture<ResponseType> result = new CompletableFuture<>();

  RequestBody requestBody = RequestBody.create(jsonb.toJson(requestPayload), MediaType.parse("application/json"));

  Request request = new Request.Builder()
    .url(url)
    .post(requestBody)
    .build();

  client
    .newCall(request)
    .enqueue(new Callback() {
      @Override
      public void onResponse(@NotNull Call call, @NotNull Response response)
      throws IOException {
        if (response.isSuccessful()) {
          result.complete(
            jsonb.fromJson(response.body().string(), resultType)
          );
        } else {
          result.completeExceptionally(new RuntimeException());
        }
      }

      @Override
      public void onFailure(@NotNull Call call, @NotNull IOException e) {
        result.completeExceptionally(e);
      }
    });

  return result;
}

The first request could then simply look like

private CompletableFuture<RequestBodyOne> makeFirstRequest(RequestBodyOne body) {
  return makeRequest(
    "https://jsonplaceholder.typicode.com/posts",
    body,
    RequestBodyOne.class
  );
}

We can also implement the second request, including the combining, in a manner like this

private CompletableFuture<CombinationBothResponses> makeSecondRequest(RequestBodyOne firstResponse) {
  return makeRequest(
    "https://reqbin.com/echo/post/json",
    new RequestBodyTwo(firstResponse.getId(), "request 2"),
    CombinationBothResponses.class
  ).thenApply(secondResponseContent ->
    new CombinationBothResponses(
      firstResponse.getUserId(),
      firstResponse.getId(),
      firstResponse.getTitle(),
      firstResponse.getBody(),
      secondResponse.success()
    )
  );
}

And using CompletableFuture::thenCompose the two requests can easily be chained like

CompletableFuture<CombinationBothResponses> combinedResponse = makeFirstRequest(requestBodyOne)
  .thenCompose(CombinationService::makeSecondRequest);
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.