Why aren't my callback methods working properly?

Question

I use AsyncRestTemplate to make resttemplate asynchronously.

These methods should wait all asyncresttemplate processes till done, And It will return reviewContent.

Problem is callback methods are not working, before the entire method works done. So I can't take proper return value of optionName and membershipGradeCode and reviewType should be included in reviewContent.

Could someone explain what am I missing now?

rev#1 Success callback methods change the state of reviewContent, Could it be a problem?

public ReviewContent getRepresentativeReviewContent(Long dealNo, Long categoryNo, String setId) {

    Optional<Map<String, Object>> review = Optional.ofNullable(boardApi.getRepresentativeReviewContent(dealNo));

    if (review.isPresent()) {
        Long memberNo = Long.valueOf(review.get().get("memberNo").toString());
        ReviewContent reviewContent  = new ReviewContent();

        ListenableFuture<ResponseEntity<Map>> optionInfo = dealApi.asyncGetDealOption(Long.valueOf(review.get().get("optionNo").toString()));
        optionInfo.addCallback(success -> {
            try {
                reviewContent.setOptionName((String) ((Map<String, Object>) success.getBody().get("data")).get("dealTitle"));
            } catch (Exception e) {
                reviewContent.setOptionName(null);
            }
        }, failure -> LOGGER.error("asyncGetDealOption", failure.getStackTrace()));

        ListenableFuture<ResponseEntity<Map>> gradeInfoOfThisMember = mktApi.asyncGetMembershipGradeOfThisMember(memberNo);
        gradeInfoOfThisMember.addCallback(success -> {
                    try {
                        reviewContent.setMembershipGradeCode((Integer) ((Map<String, Object>) success.getBody().get("data")).get("grade"));
                    } catch (Exception e) {
                        reviewContent.setMembershipGradeCode(0);
                    }
                        },
                        failure -> {
                            reviewContent.setMembershipGradeCode(0);
                            LOGGER.error("asyncGetMembershipGradeOfThisMember", failure.getStackTrace());
                        });

        ListenableFuture<ResponseEntity<ReviewType>> reviewTypeByCategoryNo = boardApi.asyncGetReviewTypeByCategoryNo(categoryNo, setId);
        reviewTypeByCategoryNo.addCallback(success -> {
                    try {
                        reviewContent.setReviewType(success.getBody());
                    } catch (Exception e) {
                        reviewContent.setReviewType(null);
                    }
                },
                failure -> {
                    reviewContent.setReviewType(null);
                    LOGGER.error("asyncGetReviewTypeByCategoryNo", failure.getStackTrace());
                });

        reviewContent.setReviewCount((Integer) review.get().get("reviewCount"));
        reviewContent.setReviewAvgScore((Double) review.get().get("reviewAvgScore"));
        reviewContent.setContents((String) review.get().get("contents"));
        reviewContent.setCreateDt((String) review.get().get("createdDt"));
        reviewContent.setUpdateDt((String) review.get().get("updatedDt"));
        reviewContent.setMemberSrl(memberNo);
        reviewContent.setTitle((String) review.get().get("title"));
        reviewContent.setAccountSrl(Long.valueOf(review.get().get("accountNo").toString()));
        reviewContent.setMemberId((String) review.get().get("memberId"));
        reviewContent.setAccountSrl(Long.valueOf(review.get().get("accountNo").toString()));

        boolean isApiExecutionDone = false;
        while (!isApiExecutionDone) {
            if (gradeInfoOfThisMember.isDone() && optionInfo.isDone() && reviewTypeByCategoryNo.isDone()) {
                isApiExecutionDone = true;
            }
        }

        return reviewContent;
    }

    return new ReviewContent();
}

Answer 1

So your problem is that the callbacks set properties on the object returned by your method. However, they are also executed asynchronously, and are not part of the done status of the Future: they are themselves executed once the Future is done, concurrently with the code in the getRepresentativeReviewContent method. Since the method returns as soon as all Futures are done, the properties aren't (all) set as they should.

Moreover, you didn't show the code for your ReviewContent object, but I'm pretty sure it doesn't declare the optionType, membershipGradeCode or reviewType fields as volatile. Since there are no barriers (such as synchronized blocks or Locks) in the method, there's no guarantee in the Java Memory Model that the values set in the callbacks (i.e. in other threads) would be seen in the thread executing the getRepresentativeReviewContent method.

Callbacks should only be used for side-effects outside of your main execution path, since it's hard to coordinate with them: you would have to use things like a CountDownLatch to make sure they have all executed, that would make the code even more complex.

Just wait for the asynchronous results in a straight-forward way (the code is untested though):

try {
    // Not sure why you need to catch Exception here?
    // If it's for flow control (absent entry in a Map), it's a bad practice.
    // Just check it instead of catching NullPointerException.
    reviewContent.setOptionName((String) 
            ((Map<String, Object>) optionInfo.get().getBody().get("data"))
                    .get("dealTitle"));
} catch (InterruptedException e) {
    Thread.currentThread().interrupt();
    LOGGER.error("asyncGetDealOption", e);
    reviewContent.setOptionName(null);
} catch (CancellationException | ExecutionException e) {
    LOGGER.error("asyncGetDealOption", e);
    reviewContent.setOptionName(null);
}

Another option is to compose the Futures, such as what can be done with Guava's Futures.transform to actually get the string you need out of the complete response, so you can just call get() on that composed Future to set your property. You'd still have to managed the errors, though.