How to deal with upsert PUT requests in a Spring Boot web application without crossing service and controller layers?

Question

I've got a pretty simple web service that I need to add a PUT endpoint for that can handle upserts. This application is primarily a middleware layer to handle logging, caching and instrumentation in front of a third-party API that doesn't do anything useful like that, so it's fairly simple.

Once the endpoint receives a request, a bit of work is done in the service layer, it's transformed a bit and forwarded to the third party API, which sends back either a 200 if the resource already existed and it was updated, or 201 if it was created. I want to do similar with my endpoint, but I'm not sure how to propagate that difference from the client, through the service layer, and to the controller.

My initial thought is to just return the status code from the controller, and check for errors and throw exceptions in the service, and otherwise return the status code from the service as well so the controller can understand if an Update occurred, or a Create.

Essentially I'm looking at three layers like this, all using Spring Boot:

@RestController
@RequestMapping("/thing")
public class ThingController {
  ...

  @PutMapping(path = "v1/thing/{thing-id}", consumes = APPLICATION_JSON_VALUE, produces = APPLICATION_JSON_VALUE)
  public ResponseEntity<String> putThing(@PathVariable("thing-id") String thingId, @RequestBody ThingPutRequest thing) {
    ... validation etc...
    HttpStatus response = thingService.upsertThing(thingId, thing);
    // And then build the response with the status code based on what was returned, to indicate created or updated
  }
}

@Service
public class ThirdPartyThingService implements ThingService {
  private ThirdPartyThingClientClient thingClient;

  ...

  @Override
  public HttpStatus upsertThing(String thingId, thingRequest thingUpsertRequest) {
     ...blablabla...
     
    return thingClient.upsertThing(thingId, transformedThingRequest);
  }
}

@Component
public class ThirdPartyThingClient implements ThingClient {
  ...

  @Override
  public HttpStatus upsertThing(String thingId, ThingRequest thing) {
    ...
    String requestUri = ...
    
    try {
      // It uses patch, and different errors return json or a string -_-;
      ResponseEntity<String> response = thirdPartyClient.patch(requestUri, thing, String.class);
      ...
      return response.getStatusCode();
    }
    catch (HttpClientErrorException.NotFound e) // and other exception handling where appropriate
    ...
  }
}

This definitely doesn't look right to me. I don't like the idea of passing status codes around through the service layer, but I'm hung up on the idea of breaking up the service entry points because it feels like putting business logic in the Controller:

  public ResponseEntity<String> putThing(@PathVariable("thing-id") String thingId, @RequestBody ThingPutRequest thing) {
    if (thingService.resourceAlreadyExists(thingId) {
      thingService.upsertThing(thingId, thing); // void return type
      // build 200 response 
    } 
    else {
      // call the same method
      thingService.upsertThing(thingId, thing);
      // build 201 response 
    }
  }

I guess this really comes down to not really having a solid understanding of how to separate logic between layers, in Spring and in general. I'd love some thoughts specific to this, but would also appreciate any suggested reading or resources in the comments =)

Answer 1

My initial thought is to just return the status code from the controller

Your service layer is not your web layer. That's why you have these separate layers.

The continuation of that thought is that your service layer should therefore not be using web-specific values to indicate its work. You're dealing with a leaky abstraction here.

Let's say your third-party API gets changed in the future and you now use ThisLibrary to achieve the same work. Are you going to force your service library to generate its own HTTP status code just to maintain the contract? Are you going to rewrite your service library's public interface because its internal implementation changed? Long story short: both are bad decisions.

The first question: Does your API actually need to distinguish between an update and a create? Just because a third-party API does so does not inherently mean that your API should do so as well.

If not, then you can just drop this behavior entirely.

If yes, then your service needs to define its own way to indicate create/update. This could be using a predetermined status, a custom result object, ... The specific implementation is up to you. Guessing at your requirements, I'd be thinking along the lines of:

Note that I'm a C# dev, but the focus is the overall approach, not the specific syntax.

public struct UpsertThingResult
{
    public UpsertThingStatus Status { get; set; }
    public string Message { get; set; }
}

public enum UpsertThingStatus 
{
    Created,
    Updated,
    ValidationError,
    Failed
}

// In your service layer:

public UpsertThingResult UpsertThing(Thing t)
{
    // ...

    return response.getStatusCode() switch {
        200 => new UpsertThingResult() { Status = UpsertThingStatus.Updated },
        201 => new UpsertThingResult() { Status = UpsertThingStatus.Created },
    };
}

// In your web layer:

var result = myService.UpsertThing(thing);

return result.Status switch {
    UpsertThingStatus.Created => HttpStatusCode.Created,
    UpsertThingStatus.Updated => HttpStatusCode.OK
};

This is just a basic example. The core intention is to ensure that your service layer does not rely on web technology to communicate its own results. By adding this additional step, you decouple your web layer from your service layer (and your third-party API), in a way that if the implementation in one layer changes, this does not inherently require you to change other layers as well.

Answer 2

My initial thought is to just return the status code from the controller

Yeah, do that!

It's a "web" service. Is it really realistic to expect that at some point in the near future this thing will be migrated to a "non-web" service?

All these layers and abstractions are completely unnecessary if they serve no real purpose at this exact moment. Depending on how complicated the logic is, whether it is used right now for something else, I would even delete the Service. Just call the other object from the Endpoint.