How to use injected DataPortal in business rules and workarounds for static methods?

[mtavares628]

Can someone give an example of how to utilize the DataPortal in a business rule for CSLA 6? Now that the DataPortal is being injected and not static, I'm not sure how to use it. So for example I have this rule:

        private class IsLinkedToClient : BusinessRuleAsync
        {
            private IPropertyInfo NameProperty;

            public IsLinkedToClient(IPropertyInfo primaryProp, IPropertyInfo NameProp) : base(primaryProp)
            {
                NameProperty = NameProp;
                InputProperties = new List<IPropertyInfo>() { PrimaryProperty, NameProperty };
                RuleUri.AddQueryParameter("Name", NameProperty.Name);
            }

            protected override async Task ExecuteAsync(IRuleContext context)
            {
                int ID = context.GetInputValue<int>(PrimaryProperty);
                string Name = context.GetInputValue<string>(NameProperty);
                if (ID > 0)
                {
                    AnswersLinkedToClientCommand cmd = await DataPortal.CreateAsync<AnswersLinkedToClientCommand>(ID, (AnswersLinkedToClientCommand.FieldEnum)Enum.Parse(typeof(AnswersLinkedToClientCommand.FieldEnum), PrimaryProperty.Name));
                    cmd = await DataPortal.ExecuteAsync(cmd);
                    if (cmd is object && !cmd.IsLinked)
                    {
                        string desc;
                        desc = string.Format("{0} selected ({1}) is no longer linked for use with clients", PrimaryProperty.FriendlyName, Name);
                        context.AddWarningResult(desc);
                    }
                }
            }
        }

How can that be rewritten in CSLA 6?

Additionally, I have concerns with no longer being able to utilize static methods in my BOs. I've been supporting a project utilizing CSLA since v3, and have been updating that project with each new iteration, but past iterations still allowed for me to utilize some older methodologies, when I couldn't quite make things fit with the newer ones. For example, a lot of my per type authorization rules needed more than just a user's role and required checking a database for additional security that may have been put on a record. To make this work, I kept utilizing the old CanGetObject static functions from v3 and passed in the necessary parameters. I would then call the static authorization methods from within the static factory methods. Here's an example:

        public static async Task<bool> CanGetObjectAsync(int clientID)
        {
            var result = default(bool);
            if (BusinessRules.HasPermission(AuthorizationActions.GetObject, typeof(Client)))
            {
                HasPermissionForClientCommand cmd = await DataPortal.CreateAsync<HasPermissionForClientCommand>(clientID, AccessPermissions.ReadAccess);
                cmd = await DataPortal.ExecuteAsync(cmd);
                result = cmd.HasPermission;
            }

            return result;
        }

        public async static Task<Client> GetClientAsync(int clientID)
        {
            var canGet = await CanGetObjectAsync(clientID);
            if (!canGet)
            {
                throw new System.Security.SecurityException("User not authorized to load this Client");
            }

            return await DataPortal.FetchAsync<Client>(clientID);
        }

While it is extra work, it is flexible enough so that I have the authorization I need on my objects. However, with the new injected DataPortal, I'm not sure how to make this work anymore, since I can no longer use the static methods. This was why I was inquiring about async authorization rules last month (#2643), because the current out of the box solution for CSLA 6 won't work in blazor which requires async dataportal methods. Is there some workaround that I can do until async auth rules are an option?

Finally, I feel like while moving to an injected DataPortal seems to be necessary, the tradeoff is losing a lot of the nice encapsulation that CSLA's BOs provided. It was nice being able create static methods so that when instantiating a business object, you knew what parameters were needed with the static factory method. Now it looks like those parameters are just a param array being called from the UI's injected DataPortal, which loses the ability to pick up bugs at compile time.

Additionally, things like additional methods that are pertinent to the BO, but outside the scope of Create/Fetch/Insert/Update/Delete can now no longer be easily encapsulated. For example on the ProjectEdit BO in your Project Tracker Project:

    public static async Task<bool> ExistsAsync(int id)
    {
      var cmd = await DataPortal.CreateAsync<ProjectExistsCommand>(id);
      cmd = await DataPortal.ExecuteAsync(cmd);
      return cmd.ProjectExists;
    }

I assume that now, that check will have to be made directly from the UI and inject the DataPortal with the ProjectExistsCommand. So instead of the user just needing to work with ProjectEdit, they have to work with the command BO directly.

This could also just be my lack of understanding with the new methodologies, and there is a way to use injection while still maintaining this encapsulation. Please let me know if there is a way to do any of the 3 things I've mentioned.

I also want to state that I'm not complaining here. I just want to know how to make this stuff work. I think CSLA is amazing, and have been using it for the past 17 years. I appreciate all the thought and work that goes into it. Hopefully, I will have a better understanding of how to make these things work once more examples are available.

[saulyt]

In the latest preview that Rocky released yesterday, the BusinessBase, CommandBase, and ReadOnlyBase classes have a GetDataPortal method. I think this may solve some of, if not all of your issues. BusinessRuleBase does not have one, but perhaps it can be passed in as a parameter. Havn't had a chance to try for myself yet.

Link to commit:
#2658 Add GetDataPortal method to support lazy loading

[rockfordlhotka]

This is a good point - there should probably be a GetDataPortal method on the rule context to support this requirement.

I've recorded this as #2682

[rockfordlhotka]

Regarding the use of static factory methods, I really need to write a blog post about this soon, because it is the biggest hurdle most people are encountering.

You can not use static factory methods - there's no safe way to make that happen. There are alternatives.

First, you can drop the use of factory methods and just use IDataPortal<T> instances in your client-side code. If your factory methods don't do anything other than just call the data portal, this is the simplest solution.

Second, you can create a factory class and put your static factory methods into that class as instance methods. This factory class will require the necessary IDataPortal<T> instances be injected into its constructor, so they are available to the methods.

Third, and this only works in WinForms or WPF, you can implement a static GetDataPortal<T> method on the App class for your app. This is a hack, in that you are relying on the fact that WinForms/WPF pre-date dependency injection, so it is possible (during app startup) to capture a static reference to the service provider, and use that to implement the static GetDataPortal<T> method. Your static factory methods can then use App.GetDataPortal<PersonEdit>() to get a data portal instance for their use. This is the lowest-effort solution for existing codebases, and I think it is a fine solution - but nobody should pretend like it is a "good thing" 😉

[mtavares628]

Thank you both for the responses. As a follow-up, why is the following not an option:

I'm using your BlazorExample for the sample. I've adjusted PersonList like so:

[Serializable]
  public class PersonList : ReadOnlyListBase<PersonList, PersonInfo>
  {
    public static async Task<PersonList> GetPersonListAsync(IDataPortal<PersonList> dataPortal)
    {
      var personList =  await dataPortal.FetchAsync();
      return personList;
    }

    [Fetch]
    private void Fetch([Inject]DataAccess.IPersonDal dal, [Inject]IChildDataPortal<PersonInfo> personInfoPortal)
    {
      using (LoadListMode)
      {
        var data = dal.Get().Select(d => personInfoPortal.FetchChild(d));
        AddRange(data);
      }
    }
  }

and then ListPersons.razor:

@page "/ListPersons"
@using BlazorExample.Shared
@inject Csla.IDataPortal<PersonList> personListPortal
@inject Csla.Blazor.ViewModel<PersonList> vm

<h1>List of People</h1>

<p class="alert-danger">@vm.ViewModelErrorText</p>

@if (vm.Model == null)
{
  <p>Loading List...</p>
}
else
{
  <p>
    <a href="editperson">Add person</a>
  </p>
  <table class="table">
    <thead>
      <tr>
        <th>Name</th>
        <th></th>
      </tr>
    </thead>
    <tbody>
      @foreach (var item in vm.Model)
      {
        <tr>
          <td>@item.Name</td>
          <td><a href="editperson/@item.Id">Edit</a></td>
        </tr>
      }
    </tbody>
  </table>
}

@code {
  protected override async Task OnParametersSetAsync()
  {
     //await vm.RefreshAsync(() => personListPortal.FetchAsync());
     await vm.RefreshAsync(() => PersonList.GetPersonListAsync(personListPortal));
     var temp = vm.Exception;
  }
}

It seems to work and allows keeping of the static methods, just passing the injected dataportal from the UI. Is there a reason why this shouldn't be done?

[rockfordlhotka]

Oh, sure. I just hadn't thought of that, but that'll work fine.

[mtavares628]

Ok, that's good to know, thanks. Just so I'm clear as well, I would need to pass in a different dataportal object for each command that I would call, correct? So, taking my CanGetObject example above, I would have to rewrite it like this:

public static async Task<bool> CanGetObjectAsync(int clientID, IDataPortal<HasPermissionForClientCommand> cmdPortal)
        {
            var result = default(bool);
            if (BusinessRules.HasPermission(AuthorizationActions.GetObject, typeof(Client)))
            {
                HasPermissionForClientCommand cmd = await cmdPortal.CreateAsync(clientID, AccessPermissions.ReadAccess);
                cmd = await cmdPortal.ExecuteAsync(cmd);
                result = cmd.HasPermission;
            }

            return result;
        }

        public async static Task<Client> GetClientAsync(int clientID, IDataPortal<Client> clientPortal, IDataPortal<HasPermissionForClientCommand> cmdPortal)
        {
            var canGet = await CanGetObjectAsync(clientID, cmdPortal);
            if (!canGet)
            {
                throw new System.Security.SecurityException("User not authorized to load this Client");
            }

            return await clientPortal.FetchAsync<Client>(clientID);
        }

There's no way to re-purpose a single dataportal to for the 2 different objects, right? What I'm getting at is that I would have to inject 2 different IDataPortals in my razor component and then pass each one to retrieve my BO. I just want to make sure that there isn't a method where I would only have to inject a single IDataPortal.

[swegele]

@mtavares628 Many of us will be going through the same questions and struggles as this is a big shift (reminiscent of when we went from v3 to v4 - remember that?) Anyway I thought I'd point you to my discussion that I just posted giving a progress update and some working ideas...hoping maybe it will help you.

See comment #2684

[TheCakeMonster]

I think you could still use static factory methods if you wanted to - but only if you pass the data portal in as a parameter. Here's an example of what you can do:

public static MyBusinessType NewMyBusinessType(IDataPortal<MyBusinessType> dataPortal, int id)
{
	return dataPortal.Create(new IDCriteria(id));
}

I have chosen to do this in order to get a lot of the tests migrated; it turned out to be quicker than the alternatives on many dozens of classes there are in the test project. This may be a step towards something else as other contributors/reviewers may frown on it, but it was a pragmatic choice to get us working tests. One step at a time!

As it happens, although it's a little odd as a pattern, I think there are some benefits to doing this:

1. BO classes remain fairly self-contained; there is only one class and one file for each BO.
2. You retain the type safety that having a dedicated method can provide; you can call the data portal directly with any combination of parameters and this may or may not match a data portal method on the object - you would only find that out at runtime.
3. You also retain the type being self-documenting in its usage; seeing the available static methods offers guidance to developers who know the object model less than perfectly.
4. It avoids tempting people to use the constructor, which now doesn't work at all - validation checks fail miserably if you try.
5. If you still use criteria classes (e.g. to help discriminate between different overloads of data access methods) then they can remain private in scope.
6. It's arguably a quicker way to convert an existing codebase that has static methods.

It's possible to get some of these benefits using other means, but so far I haven't found another way of getting as many.

Creating a separate factory class for each BO type that can be registered with DI and injected into your components should work too, but you'd have to either change the criteria classes to have a wider scope, or maybe nest the factory type inside of the BO type, which results in slightly more awkward syntax for the user of the factory type.

[swegele]

@TheCakeMonster
Those are very good points and well made. It really helps to have that when considering whether to simply add DataPortal to your current BO static factory methods or to create small separate Factory classes for each business objects.

I think there are at least two additional considerations though. Very often, within our BO static factory methods, we do some security checks based on who is logged in and we may even have to call some command object or other thing which requires more types of DataPortal. So two problems:

1. Unless I'm mistaken there is no way to get the current user without ApplicationContext. Simply having the DataPortal is not enough. So now you will also require that the ApplicationContext is passed in as one more parameter.
2. If you have to make a call to some additional Command object to check/validate anything, then now you have to pass in TWO DataPortal objects. We have no mechanism for simply getting a generic instance of a DataPortal which can be used to call multple T business objects.

It started to concern me seeing more and more CSLA-specific knowledge bleeding out of my business objects to become the responsibility of the consumer. I don't think consumers of the business objects should have to know or care what a DataPortal is (at least in general outside of perhaps configuration of a remote portal which is probably done through an installer anyway)
@mtavares628 makes a reference to this same concern:

the tradeoff is losing a lot of the nice encapsulation that CSLA's BOs provided.

Additionally, and this is the big one, if I end up in the future needing even more CSLA-ish parameters for use in my static factory methods, if I did it the way you mention, adding them becomes a breaking change to all consumers. But if you had intially created a factory class, then you could simply add an additional parameter to the DI constructor and you haven't broken anyones stuff.

You are right that I will need to make my Criteria classes (when used privately) at least internal. I hadn't thought of that.

Additionally, for my MANY automated tests I will need to go from this:

var translations = TranslationRecordInfoList().GetAll();
//do tests on translations...

to this:

//will need an instance of (IServiceProvider sp) available to use in test methods (set that up in class initialize)

var translations = sp.GetService<TranslationRecordInfoListFactory>().GetAll();
//do tests on translations...

That last change will be a real cut/paste PIA with thousands of edits...but shrug.

[TheCakeMonster]

@swegele Yes, you too make good points.

I feel your pain with regards the risk of having to make breaking changes to your codebase if you add an extra parameter. DI does help overcome that problem, for sure. It is very nice in that regard. However, there might be a few other ways of sidestepping that problem:

1. You could make the data-access-y parameter you pass to your static factory method a compound object, perhaps a single, custom DataAccessFactory type? This type could expose ApplicationContext and a generic method that allows construction of an IDataPortal. I did raise the idea of creating an implementation of an existing (but unused) interface called IDataPortalFactory in issue Should Csla 6.0 expose a client-side IDataPortalFactory interface and implementation? #2676, but even if we did that, I'm not sure it would be right for it to expose ApplicationContext. As it happens, it's also not something I feel I'd need for the API of my business objects; I really intended that factory for use in the UI Control layer.

2. I can't recall having had anything other than single line factory methods on my BOs in the past. That's not to say I have never done it, but I don't remember doing so at the moment. Csla already supports type-level authorization rules which I've relied on to do initial auth checks, so I haven't ever felt the need to do anything besides the data access call itself. Any additional logic I apply in the data access method, which I expect to be running on a trusted server in more/most architectures, which is probably less true of the client-side code.

3. I think if I were faced with having to add another data access call into a factory method I'd be tempted to wonder if the design of my class was right - at least in terms the new usage of it. I tend to want my classes to do one thing really well; if I'm having to consider changing it to add more data access then I'd be tempted to ask if it would be better off creating a different class that can more simply fulfil the new need using the fairly simple one-call-per-factory pattern - which, as it happens, I tend to code generate and then never touch again.

4. I tend to think of data portal calls as extremely expensive - not because they actually are, but because they could be if they were to call into a different appdomain, across a network - perhaps a long network. A Csla data portal call across a slow link from Mars to Earth, anyone? No? Ah, just me then. Hehehe. Anyway, the point is that I want to do that transition only once. If there is extra work to do that involves data access, I'd rather get across the data portal once, do all of the data access work near the data store, and then get back again. Rather than initiating another data portal hop from my client, I'd instead prefer to add an IChildDataPortal parameter to the signature of my data access method, for Csla to DI on my behalf if the [Inject] attribute is applied - something that is already well supported.

5. If all else fails, tooling is surprisingly good at this sort of refactoring. Visual Studio's 'Change Signature' refactor is a handy thing to have seen demoed - I'm not sure I'd have found it myself. I haven't used Rider but I gather its refactorings are very good, so that is bound to offer something similar. VS Code can probably do the same using a ReSharper plugin or something. Those can only do within a solution of course; if you're in a scenario where your codebase is split across multiple solutions then that is not a one stop solution.

I'm not saying that I think the factory class idea is bad, not by any means. I am also considering doing that myself. However, I'm continuing to explore the options at the moment, weighing up all of the pros and cons. Having the criteria classes private is a nice encapsulation that reduces the possibility that the object model can be misused, so I'm seeing if it's possible to retain that. I also like the discoverability of the static method approach; the developer can see from the API that the class exposes its own contracts about what it does - and does not - support. The factory classes do that too - but only if someone thinks to look for the factory type.

It also doesn't help that object factories is already a thing in the Csla world - used on the server. I haven't done that myself as I am a fan of encapsulation, and the factories can sometimes force you to tweak the boundaries to make them work. However, I wouldn't want to have confused developers on forums mixing the two ideas up.

Oh and yeah, I feel the pain on the unit test changes too. Been there, just submitted that PR this week! Fortunately, the test suite wasn't as big as it might be for a large corporate system!

No right and wrong answers for me, just trade-offs. I'm enjoying and appreciating the discussion, thanks.

[swegele]

START EDIT 2:
**PLEASE IGNORE EVERYTHING I SAID IN THIS ORIGINAL COMMENT AND EDIT 1 ** See my latest comment below.
END EDIT 2:

@TheCakeMonster Following up our discussion about good ways to update from CSLA 5 to 6...here is real feedback from experience that might hopefully help others. I actually STARTED down the route of making a factory for each business class. It would have worked but the impact would have been bigger/more complex on my apps that consume the BL.

I eventually decided to keep the static methods on my business objects and NOT use factories. Now I explicitly pass ApplicationContext to all of them which spiders off into almost every nook and crany of the BL. So it's a LOT of cut/paste because ApplicationContext is literally EVERYWHERE now. I'm up to about 6000 presses of ctrl-V or so :-) and I've got another 2000 to go. That is a lot of changes but it is the same simple change everywhere and very easy to do/fix.

This passing ApplicationContext around works because of a nice little method called ApplicationContext.CreateInstanceViaDI<...>();. This method allows me to get whatever I need whenever I need. Thus I'm not worried (like I was in the above comment) about ending up needing more parameters in future because that DI method covers most everything I might end up needing.

I'm using base classes to give all my business objects nice helper methods to get the things I need in every business object:

[Serializable()]
public abstract class JTRootBusinessBase<T> : JTCommonBusinessBase<T>
    where T : JTRootBusinessBase<T>
{
    protected static IDataPortal<T> GetDataPortal(ApplicationContext appContext)
    {
        var portalFactory = appContext.CreateInstanceViaDI<DataPortalFactory>();
        var portal = portalFactory.GetPortal<T>();
        return portal;
    }

    protected static T CslaSafeConstructor(ApplicationContext appContext)
    {
        var portal = GetDataPortal(appContext);
        return portal.Create(new BaseOnlyEmptySemaphoreObject());
    }

    protected JTDataAdapterManager GetDataManager()
    {
        return Utilities.GetDataManager(ApplicationContext);
    }

    [RunLocal]
    [Create]
    private void Create(BaseOnlyEmptySemaphoreObject emptySemaphore)
    {
    }
}

Anyway hope this helps someone in their deliberations during upgrade.

EDIT 1:
Forgot the implementation of that DI helper method that wraps Rocky's

public static class ApplicationContextExtensions
{
    /// <summary>
    /// Convenience extension method on <see cref="ApplicationContext"/> which calls <see cref="ApplicationContext.CreateInstanceDI(System.Type, object[])"/>
    /// </summary>
    /// <param name="appContext"></param>
    /// <returns></returns>
    public static T CreateInstanceViaDI<T>(this ApplicationContext appContext)
    {
        return (T)appContext.CreateInstanceDI(typeof(T));
    }
}

[swegele]

Well things were looking good, meaning it was compiling and it 'felt right', but I had not yet been able to convert my automated tests and to run them. After converted many hundreds of business objects using pattern above and then upon actual testing, I realized that this design is flawed which heavily used the ApplicationContext property of business objects so that I could keep my static Get/Create/Delete methods where they were on each business object. The only change to all the objects was to add the ApplicationContext as a parameter to all of those. Then each business object could produce that parameter from it's own property. (that was the theory).

BUT this only works if you do NOT use the feature where LocalProxy creates a new scope to isolate logical server side activities (see issue #2778 for discussion) of that feature. If you design things this way then you MUST be able to always have every reference of ApplicationContext be ONLY ONE REFERENCE. Then the underlying Csla framework stuff works correctly (like how it sets ApplicationContext.LogicalExecutionLocation and more).
It's debatable - should this be a valid option (meaning you'd need to set the property LocalProxyOptions.CreateScopePerCall = false and continue to use ApplicationContext safely within one and only ever one scope...or

Given modern stateful UIs that can be asynchronous (like Blazor/WPF etc.), then I'm thinking the only proper way to use Csla 6 is to:

• ALWAYS and ONLY access things like ApplicationContext or IDataPortal or IChildDataPortal etc. through DI or where directly implemented like the new GetDataPortal<C> methods that were added recently.
• Make factories for your business classes and move all previous static Get/Create/Delete methods to non-static methods on these
• Only get your business objects through the said factories
• Get your factories and any other stuff safely through DI as follows:
  1. From the UI layer itself (e.g. [Inject] in a blazor component
  2. From an [Inject] parameter popping out of a DataPortal method

It is arguably better that Csla 6 should simply not expose the ApplicationContext property and thus formally steer everyone towards the proper use of modern DI, closing any alluring loopholes that are just too tempting for some of us. I'm raising this as an issue in #2808 so this discussion thread can die down if it needs to.