Refactoring of a client calling a Rest Service
https://softwareengineering.stackexchange.com/questions/377202
-
07-02-2021 - |
Question
I have a class that is responsible for calling a service and returning a response.
The contract is like this
interface ServiceClient {
ServiceResponse process(ServiceSubmissionParams params);
}
ServiceSubmissionParams
and ServiceResponse
are simple POJOs that are part of the application calling the service (and not part of Service API).
ServiceSubmissionParams
has a bunch of data members that are needed to call an API of the service and ServiceResponse
has data that is interpreted from the service response.
The implementation class is like this
class ServiceClientImpl implements ServiceClient {
private Client client; //JAXRS client
ServiceResponse process(ServiceSubmissionParams params) {
//1
HttpHeaders headers = createHeaders(params);
//2
Response response = //call service with the above headers
//3 - Build reponse
if (response.getStatusInfo().getFamily() == Response.Status.Family.SUCCESSFUL) {
// Build ServiceResponse
}
//4
throw handleErrorResponse(response);
}
HttpHeaders createHeaders(ServiceSubmissionParams params) {
//Builds HttpHeaders from ServiceSubmissionParams fields
}
RuntimeException handleACAPIErrorResponse(Response response) {
/*
Interprets the response to throw a (custom) subclass of RuntimeException
if response == 503, return ServiceUnavailableException
if response == 408, return RequestTimeoutException
....
if response == 500, return ServerException
else return UnknownException
*/
}
}
The reason for throwing specific (or different) RuntimeExceptions is that based on the type some gets retried (ServiceUnavailable or RequestTimeout) while others don't.
My question:
I feel that this class does a lot of work. Prepare headers, call service, interpret response. So, does it violate SRP? Or does it do only job considering the level of abstraction of taking a ServiceSubmissionParams
and returning a ServiceResponse
.
More than a violation of SRP, I need to change this class whenever I need to handle an error response from the service i.e, I need to read the response body in case of 500 or 400 response code and interpret it to populate a ServiceResponse
. So, should I consider splitting this class into multiple classes?
Solution
There are usually different ways to look at the responsibilities a class has, in the context of SRP. If you consider that its responsibility is "encapsulate REST service calls, so that clients of this class do not have to worry about HTTP & co.", which is OK in my book even if rather broad, then having a single class is no big deal -- provided this is not a huge API resulting in a client that is hundreds if not thousands of lines long.
Remember that SRP is all about coupling reduction, but code should remain cohesive, i.e. related elements should be close to one another.
If the class is so big that working with it is difficult, then it makes sense to look into splitting it. In that case, the responsibility described above becomes that of the module (I talk here of a conceptual module, not a Java 9 one) the classes will be part of, with each class inside having a more precise responsibility.
With the problem you have of having to manage many different responses, one way to do it would have to define a ResponseHandler
interface with implementations that each manage a single case, and have your client implementation class be configured to use a given ResponseHandler
depending on the response status (ideally configured in a Map<HttpStatus, ResponseHandler>
so that you do not have to manage a chain of ifs
in the code, or in a List|Set<ResponseHandler>
with each handler defining a canHandle(Response)
method if determining which one to use is not as simple as just checking the status code). That way managing a new case would be as simple as adding a new ResponseHandler
implementation and updating the list of objects used by ServiceClientImpl
.