Improve IsSavable performance

[StefanOssendorf]

As shown in the diff I've changed the order in which the check is done to avoid calling BusinessRules.HasPermission() when it's clear it's already not savable by the other 3 properties.

The results on my machine are (tested with the BusinessBase implementation):

BenchmarkDotNet v0.13.12, Windows 11 (10.0.22631.3593/23H2/2023Update/SunValley3)
AMD Ryzen 7 5800X, 1 CPU, 16 logical and 8 physical cores
.NET SDK 8.0.300
  [Host]     : .NET 8.0.5 (8.0.524.21615), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.5 (8.0.524.21615), X64 RyuJIT AVX2

Method	Mean	Error	StdDev	Gen0	Allocated
CurrentImplementation	96.835 ns	0.3952 ns	0.3300 ns	0.0124	208 B
NewImplementation	1.313 ns	0.0112 ns	0.0105 ns	-	-

The difference is only within nano seconds but I think with that easy change and it's probably called a lot by some users it's worth it. And it does not make the code over complicate or the like.

The test setup is: I copy & pasted the old implementation changed the property name and used the new implementation. My benchmark code is the following:

[MemoryDiagnoser]
public class IsSavableBenchmark
{
  private SavableBenchmarkBO _bo;
  private ServiceProvider _serviceProvider;

  [GlobalSetup]
  public void Setup()
  {
    _serviceProvider = new ServiceCollection().AddCsla().BuildServiceProvider();
    _bo = _serviceProvider.GetRequiredService<IDataPortal<SavableBenchmarkBO>>().Create();
  }

  [GlobalCleanup]
  public void Cleanup()
  {
    _serviceProvider.Dispose();
  }

  [Benchmark]
  public bool CurrentImplementation() => _bo.IsSavable;

  [Benchmark]
  public bool NewImplementation() => _bo.IsSavableImproved;

}

[Serializable]
public class SavableBenchmarkBO : BusinessBase<SavableBenchmarkBO>
{
  public static PropertyInfo<string> FirstNameProperty = RegisterProperty<string>(nameof(FirstName));
  [MaxLength(25)]
  public string FirstName
  {
    get { return GetProperty(FirstNameProperty); }
    set { SetProperty(FirstNameProperty, value); }
  }

  public static PropertyInfo<string> LastNameProperty = RegisterProperty<string>(nameof(LastName));
  [Required]
  [MaxLength(25)]
  public string LastName
  {
    get { return GetProperty(LastNameProperty); }
    set { SetProperty(LastNameProperty, value); }
  }

  public static PropertyInfo<string> HomeTelephoneProperty = RegisterProperty<string>(nameof(HomeTelephone));
  public string HomeTelephone
  {
    get { return GetProperty(HomeTelephoneProperty); }
    set { SetProperty(HomeTelephoneProperty, value); }
  }

  public static PropertyInfo<string> MobileTelephoneProperty = RegisterProperty<string>(nameof(MobileTelephone));
  public string MobileTelephone
  {
    get { return GetProperty(MobileTelephoneProperty); }
    set { SetProperty(MobileTelephoneProperty, value); }
  }

  protected override void AddBusinessRules()
  {
    base.AddBusinessRules();
    // Add additional rules for warning severity level
    var rule = new Csla.Rules.CommonRules.MinLength(LastNameProperty, 2, "Last name is quite short!");
    rule.Severity = Csla.Rules.RuleSeverity.Warning;
    BusinessRules.AddRule(rule);

    // Add additional rules for information severity level
    rule = new Csla.Rules.CommonRules.MinLength(FirstNameProperty, 2, "First name is a bit short");
    rule.Severity = Csla.Rules.RuleSeverity.Information;
    BusinessRules.AddRule(rule);
  }

  [Create]
  private void Create() {
    FirstName = new string('a', 30);
    LastName = "a";
    HomeTelephone = "asdas";
    MobileTelephone = "asdas";
  }
}

[rockfordlhotka]

The problem I see is that this could allow someone without permissions to trigger business rules or other checks to run.

Though maybe not, as the saveable check is just looking at pre-set boolean values maybe?

[ossendorf-at-hoelscher]

Yes. The IsSavable only looks at bools to determine the state of the object. And if a simple get of a property can trigger rules to run I would be surprised.