CSLA 6 Upgrade Doc - Thoughts

[TheCakeMonster]

I thought I'd put this as a discussion, as it's ideas rather than firm recommendations.

Non-DI
I was reading the advice on the non-DI scenario, and I wondered whether suggesting a further step towards the DI solution might be an idea. The advice is to make a global instance of ApplicationContext available and use that to create instances of the data portal. However, I wondered if it might be better for them to also create a global instance of IDataPortalFactory, and use that for all of the object creation operations instead? That way, their code would be much closer to what they would see in samples that use DI. I wondered if this might make the mental divide between the two worlds smaller.

CORS
The example policy associated with changing CORS configuration for Blazor WASM seems to turn CORS off completely. While that would be the easiest way for people to get up and running, it's not a very secure configuration - it risks opening up the host site to XSRF attacks and so on. Generally, people should add allowed origins only as necessary. Should we perhaps link to a separate document containing more detail on that, or otherwise try to explain it with a note in the same document?

My understanding is that, if the site is published by the same site as contains the web API or exposes the data portal, then the policy doesn't need to be changed at all. I wonder if that's the most likely situation for most people - but that's pure speculation on my part.

UseTextSerialization
I think I saw somewhere that Blazor WASM has been upgraded and supports binary data as well as of .NET 6. It might be that the overhead of forcing text serialization is no longer required. I'm not sure this is true and I can't find a reference to the change at the moment, but I thought I'd mention it in case it rang a bell with anyone.

Static Factory Methods
I think moving away from static factory methods on business objects was probably suggested in prior versions, but is likely to be a stumbling block for many more people this time around. I can't see anything in the document about this; is this something we should give extra guidance on? Whilst we are guiding people towards DI in the rest of the document, they may not appreciate the impact if we don't call it out.

Of course, which of several alternative patterns we suggest instead is the tricky bit!

[rockfordlhotka]

These are good comments @TheCakeMonster, and I've tried to address most of them here.

I need to test the UseTextSerialization idea - it would be really nice to totally eliminate that entire behavior!

[mtavares628]

I'd like to just throw my two cents in regarding recommending people move away from static factory methods. I have 2 arguments for keeping them.

First, I think having people call the dataportal directly loses the encapsulation that CSLA objects have always provided. With the static methods you are able to see what parameters are necessary to pass in at compile time rather than using the dataportal directly with a paramarray. I think the convention of having users pas that extra parameter of a dataportal or dataportal factory is worth the trade off of being able to see what you need to pass into a fetch or create with intellisense.

Second, and I know this is more of an edge case, but the static methods allow for people to work around some of the current authorization limitations. For example if you need to perform some authorization against the database, in blazor you can't rely on just using the current built in auth rule mechanism because async auth rules are not currently supported. So as a work around you would need to write your own authorization check with a command object. With the static methods you can encapsulate this call within the static method and generate a security exception if the auth check doesn't pass. Otherwise, if using the dataportal call directly, the consumer needs to remember that they need to perform the auth check themselves with a different dataportal.

I just think keeping the static methods puts the ownness on the object to make sure that authorization passes, and that the correct parameters are passed into the dataportal instead of making the consumer worry about performing their own custom auth checks, and that they are passing parameters in the right order by making the dataportal calls directly.

I guess what I'm getting at is that maybe it should be pointed out that there are still benefits to using the static methods in some cases as opposed to recommending getting rid of them in favor of the direct dataportal calls.

[rockfordlhotka]

Those are very good points.

I still think static methods are not the best option.

You have me thinking though, about some generic factory type or base type that would support creating/fetching/deleting root objects.

Just the start of a thought.

[TheCakeMonster]

@mtavares628 Yes, I agree with your concerns.

I'm not much of a fan of using the data portal directly as shown in a lot of samples. This gives developers very little help (such as from IntelliSense) as to the required parameters in any situation. It also increases the risk of bugs, as the compiler is giving no help with data type checking or parameter counts. Finally, I think it makes refactoring the code a bit more difficult, as finding and changing all of the call sites is more difficult if you need to add extra parameters as the system evolves. Generally, the change cannot be achieved using built-in refactoring support in the IDE.

You don't have to retain static factory methods to get most of the same benefits they provide. The use of instantiable factory classes also resolves the majority of the downsides of direct use of the data portal. However, factory classes may also be imperfect. The potential downsides I see to factory classes are twofold: the increase in the number of items injected into constructors of consuming code, and the change in scope needed for some classes (including criteria objects.)

I can imagine the constructors of ViewModels or UI components getting more complex as more types are used in the system. If you need to use a few different types in one ViewModel/UI component and need to inject a factory type for each, then that starts to grow the number of items injected into the constructor. However, this does correctly reflect the coupling between the caller and the CSLA classes in use. Seeing too many items injected may lead developers to consider that their business objects are not achieving the intended simplicity - that the model might be wrong, and that other, composite/aggregate types may be required. Alternatively, it might indicate that the consuming code is not sufficiently segregated - that the consumers have too many responsibilities. These may be benefits in some ways, rather than just drawbacks.

Personally I still favour using criteria objects for getting parameters into data access methods. This is partially historical; in older versions of CSLA only a single parameter was supported, and therefore criteria objects were a must in a lot of situations. This is no longer the case because CSLA now supports passing multiple parameters. However, I like the fact that I can definitively discriminate between data access method overloads using different criteria types, so I am gaining easier extensibility by continuing to use criteria objects. To use factory classes I need to change the scope of my criteria objects from private to internal - and perhaps change their nesting - but that's not the end of the world.

Static factory methods have been frowned upon by some of my developers because their use made mocking the CSLA classes nigh impossible in unit tests of consuming code. They considered this a weakness of CSLA, to the point they claim CSLA is 'not modern' and should not be used. However, I continue to have a different view of testing. I am of the opinion that, in general, we should not be creating tests on classes in complete isolation. Thinking that the item under test should be a single class in every instance makes a system more brittle. By splitting up our tests to be against single, isolated classes we risk failing to test the real-world behaviour in sufficient depth - to include the interactions between objects. Furthermore, tests written this way actively make changing the system more difficult - because the tests break if we change any exposed members, and we are encouraging more members to be exposed so that they can be tested.

I think it's important to be able to mock out the environment of course, but not every single other class. Tests should reflect real-world usage as closely as possible, and that includes testing the interactions between objects. That means that when testing a ViewModel - one of the areas of a user-centric system in which I find tests to be most valuable - I want to test against the CSLA object themselves. The thing I want to mock is the data access they do (the call to the database or external API being used) instead of the CSLA object.

I am probably going to continue to use static factory methods for the time being, passing in an instance of IDataPortalFactory. However, I do consider this to be a very tough call, and I definitely remain open to switching to factory classes; this is something I will continue to review as my use of CSLA 6 continues to evolve.