0
\$\begingroup\$

I'm integrating with a 3rd party vendor that has Users and Organizations in it. My tool is a user administration tool that allows us to store information in our local database about the Users and Organizations that we manage in the 3rd party vendor. So the Users and Organizations are stored in the 3rd party vendor as well as our database (we act as the middle man sort of). I'm at a crossroads with some ideas and would like to hear what others thought of the following problem and my solution.

Before I dive in, we're using Grails 2.4.4

One use case for the tool is creating a User. We create the user in the 3rd party vendor via an API call, and then we store information about that user in our database. There are many uses cases like this and there are 3 fields that are always in common between them, an API Token a Data Center ID and an Organization ID. These 3 fields are required for just about every API call we make to our 3rd party vendor and most of the Controller Actions result in an API call being made. For this reason, I had the idea to make a "Common" Command Object with these 3 fields on it that can be extended by another Command Object.

@Validateable
class CommonRequestCommand {

    String apiToken
    String dataCenterId
    String orgId

    static constraints = {

        apiToken nullable: true
        dataCenterId nullable: true
    }
} 

@Validateable
class UserCreateCommand extends CommonRequestCommand {

    String email
    String firstName
    String lastName

}

There is one catch to this idea. The apiToken and dataCenterId are mutable (which I believe immutability is strongly suggested by Spring). It's possible that the apiToken and/or dataCenterId fields are supplied by the client/UI, or they might not be (orgId will always be supplied from the UI form). If they are not supplied, we query our database for them. If they do not exist in our database, then we throw an error. They almost always will NOT be supplied. To do that, I change the CommonRequestCommand to override the Getter methods and look up the API Token or Data Center ID if they are not provided

@Validateable
class CommonRequestCommand {

    private String apiToken
    private String dataCenterId
    String orgId

    public String getApiToken() {

        if(apiToken == null){
            Organization org = Organization.findById(orgId)
            if(org == null){
                throw new OrganizationException("Organization not found")
            }
            apiToken =  org.apiToken
        }
        return apiToken
    }

    public void setApiToken(String apiToken){
        this.apiToken = apiToken
    }

    public String getDatacenterId(){

        if(this.dataCenterId == null){
            Organization org = Organization.findById(orgId)
            if(org == null){
                throw new OrganizationException("Organization not found")
            }
            dataCenterId = org.dataCenterId
        }

        return dataCenterId
    }

    public void setDatacenterId(String dataCenterId){
        this.dataCenterId = dataCenterId
    }
}

My Question: Do you think this is an appropriate design? I believe the biggest drawback to this is that the CommonRequestCommand is mutable.

Brainstorming a few other ways:

  1. Drop the CommonRequestCommand and inheritance of command objects and instead create an Interceptor that creates a POGO CommonRequestObject and stores it in the request scope. Then in the controller, I pass both the UserCreateCommand object and the CommonRequestObject to the service layer.
  2. Just pass the 3 fields through the params scope and don't worry about creating the CommonRequestCommand or an interceptor. Instead, every method in the service layer would have an additional 3 arguments for these fields.
\$\endgroup\$
1
\$\begingroup\$

I don't like the idea of modeling a common set of properties (or as you say "... for just about every API call..." probably a mostly common set of properties) as a superclass. IMO inheritance should be used to capture common business aspects, not common technical aspects. Or in other words, this feels somewhat like identifying a set of real world objects which have a lock, and then creating a HavingLock superclass with subclasses House, Car, Desk, ... - this simply does not sound right.

Then, the CommonRequestCommand has the ring of something that is a pure data holder. It has a set of parameters and maybe even a specific execution logic that is tied to the service you call. Tying this this to the lookup logic in your own application will make this a hard-to-test mess.

Thus, I agree that an immutable class will probably be the best, and furthermore recommend that you make that immutable class just a field in your commands. For the mutablility, you can always create a copy.

All in all:

  • put your parameters in a RequestParameters class, which is immutable and which does not contain business logic
  • for a request coming from the UI, check whether you have all parametes, look up the ones missing, and create such a RequestParametes object. This should be done in a business class in you application
  • After you created the RequestParameters object, create the concrete Command, passing the RequestParametes in.
  • Continue processing the command.
\$\endgroup\$
  • \$\begingroup\$ I like this idea, and have been leaning towards it. I think this follows my Interceptor idea. I'd "intercept" all requests made to my controllers and create the RequestParameters object at this point. Once it is created, I'll let the request continue to the controller where Grails will create the Command Object. Thanks for the input \$\endgroup\$ – Eric Mar 23 '18 at 13:13

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.