3

I'm working with an API that has many asynchronous calls and handlers. I'd like to extend these with a RESTful interface and endpoints in spring. I'm imagining the usual Controller and Service layers, where the Service probably wraps these async handlers. I've read several articles about CompletableFuture, @Async, and @EnableAsync and eventually got this working, but am not sure I'm doing it 'right' or well-designed. This post does a good job of merging the CompletableFuture and async handlers; at least, the handlers there are similar to mine.

so I have something like this interface:

interface AccountHandler {
   void onSuccess(Portfolio result);
   void onError(Throwable error);
}

and now this CompletableFuture merged with it:

class MyPortfolioCallbackHandler extends CompletableFuture<Portfolio> implements AccountHandler {
   void onSuccess(Portfolio result) {
      super.complete(result);
   }
   void onError(Throwable error) {
      super.completeExceptionally(error);
   }
}

The Service looks like this:

@Service
public class PortfolioService {

    @Async
    public CompletableFuture<Portfolio> getPortfolio() throws Exception {
        MyPortfolioCallbackHandler portfolioHandler = new PortfolioHandler();
        portfolioHandler.getPortfolio();

        return portfolioHandler;
    }
}

And the Controller is this:

@RestController
@RequestMapping("portfolio")
public class PortfolioController {

    @Autowired
    PortfolioService portfolioService;

    @GetMapping(produces = "application/json")
    public Portfolio getPortfolio() throws Exception {
        return portfolioService.getPortfolio();
    }
}

This actually works, to my surprise, after trying a few things and being confused by the CompletableFuture. But is it a good design, or done 'right'?

  • I'm confused by your PortfolioService.getPortfolio() definition. MyPortfolioCallbackHandler is a CompletableFuture but you return the result of portfolioHandler.get(). Is PortfolioHandler a CompletableFuture<CompletableFuture<Portfolio>>? I'm not sure how else this compiles. – JimmyJames Jan 28 at 16:07
  • Yes, PortfolioHandler extends CompletableFuture<Portfolio>. I've updated it. Sorry, I started writing this when it wasn't working, then tried a few things, and eventually got it working. Thanks for taking a look – user26270 Jan 28 at 16:25
  • Sorry, this doesn't make sense to me. Your method is declared to return CompletableFuture<Portfolio> (as required by the Spring) but you are returning portfolioHandler.get() which should return a Portfolio. – JimmyJames Jan 28 at 16:30
  • You're right. sorry again. I've edited the service again to only return portfolioHandler. That reflects the current state. – user26270 Jan 28 at 16:58
  • Without seeing PortfolioHandler it's hard to say whether your solution is truly async. The portfolioHandler.getPortfolio(); call looks suspicious. If you are actually doing the work in that call, you are probably not getting the benefit of async execution here. Can you show more that class, or at least that method? – JimmyJames Jan 28 at 17:20
1

I'll admit some ignorance regarding Java's implementation of CompletableFuture<T>, but in general you may introduce the threat of deadlock due to hidden synchronization calls. That particular problem is definitely evident in C# async and await task synchronization.

Hidden within your portfolioHandler.get() method is a synchronization call that is waiting for the lock to be released. Combine that with the unpredictable nature of which thread you will actually be running under in your web server's thread pool and you will always have the risk of blocking the thread that is supposed to be processing the CompletableFuture.

At least I wouldn't attempt this approach with Spring MVC.

I would look into using Spring Flux which is supposed to be Asynchronous by nature. C# handled the asynchronous thread lock risk by introducing asynchronous handlers. Matching asynchronous web infrastructure with asynchronous back-end calls should help you avoid that impedance mismatch.

|improve this answer|||||
  • While await/async are fairly easy to use, they are a pretty blunt instrument compared to how Futures work in Java. "Hidden within your portfolioHandler.get() method is a synchronization call", I'm not sure there's synchronization call there, the concurrency model around the the 'new' Java concurrency is much more sophisticated than that. Future.get() will block however. The OP's code isn't actually doing that, though. It's actually Spring that is doing the call. I'm not sure exactly what it's doing but I'd bet it's using a non-blocking approach. – JimmyJames Jan 28 at 17:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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