✨ Replace string length constraint with business logic guards, and ad…

…d unit tests (#286)

* Hero: Add name & alias guard and add tests

* Remove string length constraints

* Add migration and add ADR

* Power: add tests

* Hero: Update tests and rearrange null guard to come first

* Added TeamTests

* HeroesTests: renamed to HeroTests to match domain name

* Added Id comparison tests

docs/adr/20240415-replace-string-length-constraints-with-better-business-rules.md

@@ -0,0 +1,51 @@
+# Replace database string length constraints with better business rules
+
+- Status: accepted
+- Deciders: Matt Wicks, Matt Goldman, Chris Clement, Daniel Mackay
+- Date: 2024-04-15
+- Tags: database, domains
+
+Technical Story: https://github.com/SSWConsulting/SSW.CleanArchitecture/issues/285
+
+## Context and Problem Statement
+
+As we are moving forward to make sure that CleanArchitecture template is lean and can be used
+straightforward in modern projects, we are going to drop technical constraints such as string length in names and description
+and instead opt to add more rules which adhere more to the example business logic (e.g. a hero name and alias cannot be the same).
+
+## Decision Drivers <!-- optional -->
+
+- Make sure faster setup time for CleanArchitecture templates in modern projects
+- Reduce arbitrary technical concerns in database that does not stem from thoughtful requirements
+
+## Considered Options
+
+1. (Recommended) Replace string length constraints with non-technical requirement
+2. Drop string length constraints and do not replace them
+3. Keep string length constraints but split the constants to each domain
+
+## Decision Outcome
+
+Chosen option: "Replace string length constraints with non-technical requirement", because we want to
+keep the example of guard clauses in the code.
+
+## Pros and Cons of the Options <!-- optional -->
+
+### 1. Replace string length constraints with non-technical requirement
+
+- ✅ Guard clauses example
+- ✅ No shared constraint values
+- ❌ No example of linking domain constraints to infrastructure implementation
+
+### 2. Drop string length constraints and do not replace them
+
+- ✅ No shared constants
+- ❌ No guard clauses example
+
+### 3. Keep string length constraints but split the constants to each domain
+
+- ✅ No changes to code
+- ✅ Guard clauses example
+- ❌ Shared constraint values for different entities / across aggregate root
+- ❌ Template starts with limit to string length which might not be needed / different value needed for different projects
+

src/Domain/Heroes/Hero.cs

@@ -1,5 +1,4 @@
 using Ardalis.GuardClauses;
-using SSW.CleanArchitecture.Domain.Common;
 using SSW.CleanArchitecture.Domain.Common.Base;
 
 namespace SSW.CleanArchitecture.Domain.Heroes;
@@ -18,10 +17,8 @@ public class Hero : AggregateRoot<HeroId>
  public static Hero Create(string name, string alias)
  {
  Guard.Against.NullOrWhiteSpace(name);
- Guard.Against.StringTooLong(name, Constants.DefaultNameMaxLength);
-
  Guard.Against.NullOrWhiteSpace(alias);
- Guard.Against.StringTooLong(alias, Constants.DefaultNameMaxLength);
+ Guard.Against.InvalidInput(alias, nameof(alias), input => !input.Equals(name, StringComparison.OrdinalIgnoreCase));
 
  var hero = new Hero { Id = new HeroId(Guid.NewGuid()), Name = name, Alias = alias, };
 
@@ -45,16 +42,13 @@ public void RemovePower(string powerName)
  {
  Guard.Against.NullOrWhiteSpace(powerName, nameof(powerName));
 
- var power = Powers.FirstOrDefault(p => p.Name == powerName);
+ var power = _powers.FirstOrDefault(p => p.Name == powerName);
  if (power is null)
  {
  return;
  }
 
- if (_powers.Contains(power))
- {
- _powers.Remove(power);
- }
+ _powers.Remove(power);
 
  PowerLevel -= power.PowerLevel;
  AddDomainEvent(new PowerLevelUpdatedEvent(this));

src/Domain/Heroes/Power.cs

@@ -1,5 +1,4 @@
 using Ardalis.GuardClauses;
-using SSW.CleanArchitecture.Domain.Common;
 using SSW.CleanArchitecture.Domain.Common.Interfaces;
 
 namespace SSW.CleanArchitecture.Domain.Heroes;
@@ -14,7 +13,7 @@ public record Power : IValueObject
 
  public Power(string name, int powerLevel)
  {
- Name = Guard.Against.StringTooLong(name, Constants.DefaultNameMaxLength);
- PowerLevel = Guard.Against.OutOfRange(powerLevel, nameof(PowerLevel), 1, 10);
+ Name = name;
+ PowerLevel = Guard.Against.OutOfRange(powerLevel, nameof(powerLevel), 1, 10);
  }
 }

src/Domain/Teams/Mission.cs

@@ -1,5 +1,4 @@
 using Ardalis.GuardClauses;
-using SSW.CleanArchitecture.Domain.Common;
 using SSW.CleanArchitecture.Domain.Common.Base;
 
 namespace SSW.CleanArchitecture.Domain.Teams;
@@ -19,7 +18,6 @@ public class Mission : Entity<MissionId>
  internal static Mission Create(string description)
  {
  Guard.Against.NullOrWhiteSpace(description);
- Guard.Against.StringTooLong(description, Constants.DefaultDescriptionMaxLength);
  return new Mission { Description = description, Status = MissionStatus.InProgress };
  }
 

src/Infrastructure/Persistence/Configuration/HeroConfiguration.cs

@@ -1,6 +1,5 @@
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.Metadata.Builders;
-using SSW.CleanArchitecture.Domain.Common;
 using SSW.CleanArchitecture.Domain.Heroes;
 
 namespace SSW.CleanArchitecture.Infrastructure.Persistence.Configuration;
@@ -18,12 +17,10 @@ public void Configure(EntityTypeBuilder<Hero> builder)
  .ValueGeneratedNever();
 
  builder.Property(t => t.Name)
- .IsRequired()
- .HasMaxLength(Constants.DefaultNameMaxLength);
+ .IsRequired();
 
  builder.Property(t => t.Alias)
- .IsRequired()
- .HasMaxLength(Constants.DefaultNameMaxLength);
+ .IsRequired();
 
  // This is to highlight that we can also serialise to JSON for simple content instead of adding a new table 
  builder.OwnsMany(t => t.Powers, b => b.ToJson());

src/Infrastructure/Persistence/Configuration/MissionConfiguration.cs

@@ -1,6 +1,5 @@
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.Metadata.Builders;
-using SSW.CleanArchitecture.Domain.Common;
 using SSW.CleanArchitecture.Domain.Teams;
 
 namespace SSW.CleanArchitecture.Infrastructure.Persistence.Configuration;
@@ -18,7 +17,6 @@ public void Configure(EntityTypeBuilder<Mission> builder)
  .ValueGeneratedNever();
 
  builder.Property(t => t.Description)
- .IsRequired()
- .HasMaxLength(Constants.DefaultDescriptionMaxLength);
+ .IsRequired();
  }
 }

src/Infrastructure/Persistence/Configuration/TeamConfiguration.cs

@@ -1,6 +1,5 @@
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.Metadata.Builders;
-using SSW.CleanArchitecture.Domain.Common;
 using SSW.CleanArchitecture.Domain.Teams;
 
 namespace SSW.CleanArchitecture.Infrastructure.Persistence.Configuration;
@@ -18,8 +17,7 @@ public void Configure(EntityTypeBuilder<Team> builder)
  .ValueGeneratedNever();
 
  builder.Property(t => t.Name)
- .IsRequired()
- .HasMaxLength(Constants.DefaultNameMaxLength);
+ .IsRequired();
 
  // TODO: Check this works
  builder.HasMany(t => t.Missions)