csla icon indicating copy to clipboard operation
csla copied to clipboard

Allow parameters to DataPortal.Execute<T>

Open ajj7060 opened this issue 5 years ago • 4 comments

Is your feature request related to a problem? Please describe. With the latest Csla, you can call the DataPortal.Create/Fetch with as many parameters as you want eliminating the need for a CriteriaBase sublcass. DataPortal.Execute though still only takes an instance of the command object as its only allowed parameter, making users do a DataPortal.Create to initialize any properties which act as inputs to the command.

Describe the solution you'd like Add DataPortal.Execute<T>(params object[] parameters) which would then function just like the create/fetch.

e.g. DataPortal.Execute<MarkEmailRead>(unreadOnly, emailAddress)

Would call [Execute] private void ExecuteMarkRead(bool unreadOnly, string emailAddress, [Inject] IDbConnection conn)

Describe alternatives you've considered Add a [Create] method to command objects, and do the Execute like DataPortal.Execute(DataPortal.Create<Cmd>(param1, param2) but this feels a bit clunky and adds more coding.

Additional context Idea raised here https://github.com/MarimerLLC/csla/discussions/1771#discussioncomment-52194

ajj7060 avatar Aug 18 '20 23:08 ajj7060

@ajj7060 I'm reviewing this and thinking about it.

The philosophy of a command object is that you create/initialize the object, send it to the server, and then execute it on the server.

What you are proposing is that we don't create/initialize the object or send it to the server. Instead, we send a set of parameters to the server, create the object on the server, and then execute the object on the server.

In both scenarios, the resulting object is returned to the client for further use.

I'm not entirely sure how to do this without a breaking change, because the data portal ExecuteAsync and Execute methods expect the first parameter to be the command object instance, which is of type object. So I'm not sure we can create an overload that also accepts a param array without an ambiguous method signature?

Though I suppose maybe the method could be changed to only accept a param array, and then the method could look at the first parameter in the array to see if it is an instance of T?

rockfordlhotka avatar Apr 07 '21 21:04 rockfordlhotka

@rockfordlhotka If you want to avoid the breaking change then perhaps only accepting a param array and then if there's one and only one argument check to see if its T and proceed as today; otherwise look for a method marked [Execute] that matches the param list (ignoring [Inject] parameters, like Fetch would do). Not sure how more complex that would be for Csla implementation-wise though.

I do think ultimately DataPortal.Execute<T>(T cmd) should be obsoleted at some point though. Going back to the philosophy:

The philosophy of a command object is that you create/initialize the object, send it to the server, and then execute it on the server.

Does the bolded portion add value? It feels more like an implementation detail to me, but I could be missing something.

ajj7060 avatar Apr 16 '21 13:04 ajj7060

I think in CSLA 5 DataPortal.Execute is no longer fit for purpose. Doesn't this look like code smell?

        public static string Execute(int id)
        {
            var cmd = DataPortal.Create<MyCommandBase>(id);
            cmd = DataPortal.Execute(cmd);
            return cmd.Result;
        }

So end I end up doing this - more code smell.

        public static string Execute(int id)
        {
            var cmd = DataPortal.Create<MyCommandBase>(id);
            return cmd.Result;
        }

        [Create]
        private void DataPortal_Execute(int id)
        {
            Result = "Executed: " + id;
        }

This would be so much better:

        public static string Execute(int id)
        {
            var cmd = DataPortal.Execute<MyCommandBase>(id);
            return cmd.Result;
        }

        [Execute]
        private void DataPortal_Execute(int id)
        {
            Result = "Executed: " + id;
        }

Thank you

j055 avatar Jun 02 '21 09:06 j055

Some good discussion in thread #2923

rockfordlhotka avatar May 03 '22 02:05 rockfordlhotka

There is one edge case scenario where this is a breaking change, which is where someone has directly implemented their own IDataPortal or IDataPortalT implementation. A new member has been added to these interfaces, which will require custom implementations of the interfaces to also implement these members.

rockfordlhotka avatar Aug 28 '22 00:08 rockfordlhotka

Another possible breaking change is that if someone has code that does a fetch operation against a CommandBase type, that code will now fail, because all fetch operations are redirected to an Execute operation method on the logical server.

So if you currently have a CommandBase subclass with a fetch operation:

  [Fetch]
  private void Fetch()
  {
  }

that method will now be invoked as an execute operation. You can just add the execute operation attribute as a workaround:

  [Execute][Fetch]
  private void Fetch()
  {
  }

rockfordlhotka avatar Aug 28 '22 01:08 rockfordlhotka

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

github-actions[bot] avatar Feb 25 '23 00:02 github-actions[bot]