Spring: Creating a Mutable Common Command Object or Controller Interceptor

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
0
down vote

favorite












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.






share|improve this question

























    up vote
    0
    down vote

    favorite












    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.






    share|improve this question





















      up vote
      0
      down vote

      favorite









      up vote
      0
      down vote

      favorite











      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.






      share|improve this question











      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.








      share|improve this question










      share|improve this question




      share|improve this question









      asked Mar 22 at 22:30









      Eric S

      1363




      1363




















          1 Answer
          1






          active

          oldest

          votes

















          up vote
          1
          down vote













          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.





          share|improve this answer





















          • 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
            – Eric S
            Mar 23 at 13:13










          Your Answer




          StackExchange.ifUsing("editor", function ()
          return StackExchange.using("mathjaxEditing", function ()
          StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
          StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
          );
          );
          , "mathjax-editing");

          StackExchange.ifUsing("editor", function ()
          StackExchange.using("externalEditor", function ()
          StackExchange.using("snippets", function ()
          StackExchange.snippets.init();
          );
          );
          , "code-snippets");

          StackExchange.ready(function()
          var channelOptions =
          tags: "".split(" "),
          id: "196"
          ;
          initTagRenderer("".split(" "), "".split(" "), channelOptions);

          StackExchange.using("externalEditor", function()
          // Have to fire editor after snippets, if snippets enabled
          if (StackExchange.settings.snippets.snippetsEnabled)
          StackExchange.using("snippets", function()
          createEditor();
          );

          else
          createEditor();

          );

          function createEditor()
          StackExchange.prepareEditor(
          heartbeatType: 'answer',
          convertImagesToLinks: false,
          noModals: false,
          showLowRepImageUploadWarning: true,
          reputationToPostImages: null,
          bindNavPrevention: true,
          postfix: "",
          onDemand: true,
          discardSelector: ".discard-answer"
          ,immediatelyShowMarkdownHelp:true
          );



          );








           

          draft saved


          draft discarded


















          StackExchange.ready(
          function ()
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f190247%2fspring-creating-a-mutable-common-command-object-or-controller-interceptor%23new-answer', 'question_page');

          );

          Post as a guest






























          1 Answer
          1






          active

          oldest

          votes








          1 Answer
          1






          active

          oldest

          votes









          active

          oldest

          votes






          active

          oldest

          votes








          up vote
          1
          down vote













          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.





          share|improve this answer





















          • 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
            – Eric S
            Mar 23 at 13:13














          up vote
          1
          down vote













          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.





          share|improve this answer





















          • 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
            – Eric S
            Mar 23 at 13:13












          up vote
          1
          down vote










          up vote
          1
          down vote









          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.





          share|improve this answer













          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.






          share|improve this answer













          share|improve this answer



          share|improve this answer











          answered Mar 23 at 8:23









          mtj

          2,675212




          2,675212











          • 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
            – Eric S
            Mar 23 at 13:13
















          • 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
            – Eric S
            Mar 23 at 13:13















          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
          – Eric S
          Mar 23 at 13:13




          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
          – Eric S
          Mar 23 at 13:13












           

          draft saved


          draft discarded


























           


          draft saved


          draft discarded














          StackExchange.ready(
          function ()
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f190247%2fspring-creating-a-mutable-common-command-object-or-controller-interceptor%23new-answer', 'question_page');

          );

          Post as a guest













































































          Popular posts from this blog

          Greedy Best First Search implementation in Rust

          Function to Return a JSON Like Objects Using VBA Collections and Arrays

          C++11 CLH Lock Implementation