From ca63ca1cba4bc0e9659c543c961bd6b46ebb7874 Mon Sep 17 00:00:00 2001 From: Jason Naylor Date: Fri, 28 Aug 2026 15:22:50 -0700 Subject: [PATCH] Add agent skills for the five common model tasks Adds .claude/skills/ with step-by-step guides for adding a property, adding a class, adding a virtual property, writing a data migration, and writing tests. Each was checked against current source rather than carried over as drafted, and the following were wrong: - The CellarModule table named a "CellarModule" id that does not exist and put Notebook at num 24. The real ids are Cellar 0, FeatSys 2, Scripture 3, Notebk 4, Ling 5, LangProj 6. - Code generation was described as emitting StructureMap registrations. The container is Microsoft.Extensions.DependencyInjection since #393. - Change history entries were said to go below the existing ones. The list in MasterLCModel.xml runs newest first. - DataMigration7000072.cs was cited as the example of splitting logic into private helpers. It has none; 7000065 does. - Partial interface extensions were shown in a new ILexEntryExtensions.cs. InterfaceAdditions.cs already holds about eighty of them. - Virtual FLIDs were described as starting at 20,000,000. They start at 20,000,001, are assigned in attribute construction order, and are capped at 30,000,000, so they must never be persisted or hard-coded. Version numbers in the worked examples are now marked as illustrative, with instructions to read the current version from MasterLCModel.xml. The FLEx Bridge metadata cache obligation from WARNING 4 is stated where the version gets bumped, build commands carry -m:1, and the test data directory is named rather than described. Co-Authored-By: Claude Opus 5 (1M context) --- .claude/skills/adding-a-new-class/SKILL.md | 177 +++++++++++++++ .claude/skills/adding-a-property/SKILL.md | 201 +++++++++++++++++ .../skills/adding-a-virtual-property/SKILL.md | 162 ++++++++++++++ .../skills/writing-a-data-migration/SKILL.md | 204 ++++++++++++++++++ .claude/skills/writing-tests/SKILL.md | 197 +++++++++++++++++ 5 files changed, 941 insertions(+) create mode 100644 .claude/skills/adding-a-new-class/SKILL.md create mode 100644 .claude/skills/adding-a-property/SKILL.md create mode 100644 .claude/skills/adding-a-virtual-property/SKILL.md create mode 100644 .claude/skills/writing-a-data-migration/SKILL.md create mode 100644 .claude/skills/writing-tests/SKILL.md diff --git a/.claude/skills/adding-a-new-class/SKILL.md b/.claude/skills/adding-a-new-class/SKILL.md new file mode 100644 index 00000000..8f199f8c --- /dev/null +++ b/.claude/skills/adding-a-new-class/SKILL.md @@ -0,0 +1,177 @@ +--- +name: adding-a-new-class +description: Add an entirely new class to MasterLCModel.xml, including module placement, class numbering, owner property, version bump, data migration, code regeneration, and tests. Use when the user asks to add a new class, entity, or object type to the LCM data model. +--- + +# Adding a New Class to the Model + +This guide covers adding an entirely new class to `MasterLCModel.xml`. This is less common than adding properties and more involved -- it touches the model, requires a migration, may need an owner property on an existing class, and may need hand-written partial class logic. + +## Prerequisites + +- Read `AGENTS.md`, and the `WARNING` block at the top of `src/SIL.LCModel/MasterLCModel.xml` +- Know the parent (base) class (typically `CmObject` for simple classes) +- Know whether the class requires an owner and which class will own it +- Know the properties the new class needs + +## Steps + +### 1. Determine Class Placement + +Classes live inside `` elements in `MasterLCModel.xml`. The main modules are: + +| `id` | `num` | Contains | +|------|-------|----------| +| `Cellar` | 0 | Core classes (CmObject, CmPossibility, StText, etc.) | +| `FeatSys` | 2 | Declared but empty | +| `Scripture` | 3 | Scripture classes | +| `Notebk` | 4 | Notebook classes | +| `Ling` | 5 | Linguistic classes (LexEntry, LexSense, Morph*, Wfi*, etc.) | +| `LangProj` | 6 | LangProject and related | + +Read these off the `` elements rather than trusting this table. + +Choose the module that best fits your class. Most new classes go in `Ling` (module 5). + +### 2. Determine the Class Number + +Within the module, find the highest existing `num` attribute on `` elements and use the next integer. + +The class ID (used in code as `kClassId`) is formed by combining the module number and the class number. For example, in module `5` (Ling), class number `134` would have class ID `5134`. + +### 3. Add the Class to MasterLCModel.xml + +File: `src/SIL.LCModel/MasterLCModel.xml` + +```xml + + + Description of the new class. No newlines inside para elements. + + + + + + + +``` + +Key attributes: +- `abstract`: Set to `true` if only subclasses should be instantiated +- `base`: Parent class. Use `CmObject` unless inheriting from something more specific +- `depth`: Depth in the inheritance tree from `CmObject` (0 = direct child of CmObject) +- `abbr`: Short abbreviation for the class +- `owner`: `required` (default), `optional`, or `none`. Use `none` for unowned classes like `LexEntry` + +### 4. Add an Owning Property to the Owner Class + +Unless `owner="none"`, you need a property on the owning class that references the new class. Find the owner class in the XML and add an owning property: + +```xml + + + Owns instances of NewClassName. + + +``` + +### 5. Increment the Model Version + +Read the current `version` attribute on `` and set it to the next integer. The +examples here use 7000072 -> 7000073; substitute what you actually read. + +Add a change history entry at the *top* of the list, directly under the `Change History:` +line, since the list runs newest first. + +A version bump also requires a matching update to the FLEx Bridge metadata cache, per +`WARNING 4` at the top of `MasterLCModel.xml`. + +### 6. Write the Data Migration + +Create: `src/SIL.LCModel/DomainServices/DataMigration/DataMigration7000073.cs` + +For a new class that doesn't exist in any data yet, a minimal migration suffices: + +```csharp +using System.Xml.Linq; + +namespace SIL.LCModel.DomainServices.DataMigration +{ + internal class DataMigration7000073 : IDataMigration + { + public void PerformMigration(IDomainObjectDTORepository repoDto) + { + DataMigrationServices.CheckVersionNumber(repoDto, 7000072); + // New class added to model. No existing data to migrate. + DataMigrationServices.IncrementVersionNumber(repoDto); + } + } +} +``` + +If the new class needs default instances created (e.g., a new possibility list), create them in the migration using `DataMigrationServices.CreatePossibilityList()` or raw XML construction. See `DataMigration7000069.cs` for examples of creating new lists and objects. + +### 7. Register the Migration + +File: `src/SIL.LCModel/DomainServices/DataMigration/LcmDataMigrationManager.cs` + +```csharp +m_individualMigrations.Add(7000073, new DataMigration7000073()); +``` + +### 8. Rebuild + +``` +dotnet build -m:1 --configuration Release +``` + +The code generator will produce: +- A `NewClassNameTags` constants class (class ID, field IDs) +- An `INewClassName` interface +- An `INewClassNameFactory` factory interface and implementation +- An `INewClassNameRepository` repository interface and implementation +- A concrete `NewClassName` class in `DomainImpl/GeneratedClasses.cs` +- Registrations in `GeneratedServiceLocatorBootstrapper.cs`, as + `Microsoft.Extensions.DependencyInjection` singletons: the concrete type, plus an + interface alias resolving to the same singleton + +### 9. Add Hand-Written Extensions (if needed) + +If the class needs business logic, create a partial class in `src/SIL.LCModel/DomainImpl/`: + +```csharp +namespace SIL.LCModel.DomainImpl +{ + internal partial class NewClassName + { + // Virtual properties, convenience methods, overrides, etc. + } +} +``` + +Place it in the appropriate `Overrides*.cs` file or create a new one if it doesn't fit existing files. + +### 10. Update BootstrapNewLanguageProject (if needed) + +If the new class needs default instances in every new project, update `src/SIL.LCModel/DomainServices/BootstrapNewLanguageProject.cs` to create them. + +### 11. Write Tests + +Create migration tests (use the `writing-a-data-migration` skill) and API tests using `MemoryOnlyBackendProviderTestBase` (use the `writing-tests` skill). + +## Checklist + +- [ ] Class added to correct `` in `MasterLCModel.xml` +- [ ] Unique `num` within the module +- [ ] `base` class set correctly +- [ ] `owner` attribute set (or left as default `required`) +- [ ] Owning property added to the owner class (unless `owner="none"`) +- [ ] `version` attribute incremented on `` +- [ ] Change history entry added at the top of the list +- [ ] FLEx Bridge metadata cache updated to match the new model number +- [ ] Migration class created and registered in `LcmDataMigrationManager` +- [ ] Build succeeds and code regenerates correctly +- [ ] Hand-written partial class added if business logic needed +- [ ] `BootstrapNewLanguageProject` updated if default instances needed +- [ ] Tests written +- [ ] Generated files NOT manually edited diff --git a/.claude/skills/adding-a-property/SKILL.md b/.claude/skills/adding-a-property/SKILL.md new file mode 100644 index 00000000..1e778fdc --- /dev/null +++ b/.claude/skills/adding-a-property/SKILL.md @@ -0,0 +1,201 @@ +--- +name: adding-a-property +description: Add a new persisted property to an existing class in MasterLCModel.xml, including model version bump, data migration, code regeneration, and tests. Use when the user asks to add a field, property, or attribute to an LCM model class. +--- + +# Adding a Property to an Existing Class + +This guide covers adding a new persisted property to an existing class in the LCM model. This is one of the most common and most dangerous changes -- it touches the XML model, requires a data migration, and triggers code regeneration. + +If you need a computed/derived property that is NOT persisted, use the `adding-a-virtual-property` skill instead. + +## Prerequisites + +- Read `AGENTS.md`, and the `WARNING` block at the top of `src/SIL.LCModel/MasterLCModel.xml` +- Know the target class name (e.g., `LexSense`) +- Know the property type (`basic`, `owning`, or `rel`) and signature + +## Steps + +### 1. Edit MasterLCModel.xml + +File: `src/SIL.LCModel/MasterLCModel.xml` + +Find the target class and add the property inside its `` element. Choose the next available `num` for that class (check existing properties). + +**Basic property example** (adding a `MultiString` field): +```xml + + + Description of the field. No newlines inside para elements. + + +``` + +**Owning property example** (adding an owning sequence): +```xml + + + Description of owned objects. + + +``` + +**Reference property example** (adding a reference collection): +```xml + + + Description of referenced objects. + + +``` + +Property type signatures for ``: +- `Integer`, `Boolean`, `String`, `Unicode`, `MultiString`, `MultiUnicode` +- `Time`, `GenDate`, `Binary`, `Guid`, `TextPropBinary` + +Cardinality values for `` and ``: +- `atomic` -- zero or one target +- `seq` -- ordered list +- `col` -- unordered collection + +### 2. Increment the Model Version + +In the same file (`MasterLCModel.xml`), read the current `version` attribute on the root +`` element and set it to the next integer. The examples here use 7000072 -> +7000073; substitute what you actually read. + +```xml + +``` + +Add a change history entry at the *top* of the list, directly under the `Change History:` +line. The list runs newest first. + +```xml + DD Month YYYY (7000073): Added NewFieldName to ClassName. Brief description. +``` + +A version bump also requires a matching update to the FLEx Bridge metadata cache, per +`WARNING 4` at the top of `MasterLCModel.xml`. + +### 3. Write the Data Migration + +Create: `src/SIL.LCModel/DomainServices/DataMigration/DataMigration7000073.cs` + +**For new optional properties with safe defaults (most common case)**, existing data doesn't need modification. But you still need the migration class: + +```csharp +// Copyright (c) YYYY SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +namespace SIL.LCModel.DomainServices.DataMigration +{ + internal class DataMigration7000073 : IDataMigration + { + public void PerformMigration(IDomainObjectDTORepository repoDto) + { + DataMigrationServices.CheckVersionNumber(repoDto, 7000072); + // New optional property with safe default; no data changes needed. + DataMigrationServices.IncrementVersionNumber(repoDto); + } + } +} +``` + +**IMPORTANT**: If you are adding a C# value type property (int, bool, GenDate, DateTime), you MUST add an explicit XML element with the default value to every existing instance. See `WARNING 0` at the top of `MasterLCModel.xml`. Example migration that adds a default value: + +```csharp +public void PerformMigration(IDomainObjectDTORepository repoDto) +{ + DataMigrationServices.CheckVersionNumber(repoDto, 7000072); + + foreach (var dto in repoDto.AllInstancesWithSubclasses("TargetClass")) + { + var element = XElement.Parse(dto.Xml); + if (element.Element("NewBoolField") == null) + { + element.Add(new XElement("NewBoolField", new XAttribute("val", "False"))); + DataMigrationServices.UpdateDTO(repoDto, dto, element.ToString()); + } + } + + DataMigrationServices.IncrementVersionNumber(repoDto); +} +``` + +### 4. Register the Migration + +File: `src/SIL.LCModel/DomainServices/DataMigration/LcmDataMigrationManager.cs` + +Add a line in the constructor, after the last existing entry: + +```csharp +m_individualMigrations.Add(7000073, new DataMigration7000073()); +``` + +If no data changes are needed, you can use `m_bumpNumberOnlyMigration` instead: +```csharp +m_individualMigrations.Add(7000073, m_bumpNumberOnlyMigration); +``` +In this case you do NOT need to create a `DataMigration7000073.cs` file. + +### 5. Rebuild to Regenerate Code + +``` +dotnet build -m:1 --configuration Release +``` + +This triggers `GenerateModel` which regenerates all 9 `Generated*.cs` files from the updated `MasterLCModel.xml`. The new property will appear in the generated constants, interfaces, class implementations, etc. + +### 6. Add Hand-Written Logic (if needed) + +If the property needs custom logic beyond what the generator provides (computed side effects, validation, etc.), add it to the appropriate `Overrides*.cs` partial class in `src/SIL.LCModel/DomainImpl/`. + +### 7. Write Tests + +**Migration test**: Create `tests/SIL.LCModel.Tests/DomainServices/DataMigration/DataMigration7000073Tests.cs` + +```csharp +using System.Xml.Linq; +using NUnit.Framework; + +namespace SIL.LCModel.DomainServices.DataMigration +{ + [TestFixture] + public class DataMigration7000073Tests : DataMigrationTestsBase + { + [Test] + public void DataMigration7000073Test() + { + // Parse test data XML (create a matching .xml file in the test data directory) + var dtos = DataMigrationTestServices.ParseProjectFile("DataMigration7000073.xml"); + var mockMdc = new MockMDCForDataMigration(); + IDomainObjectDTORepository dtoRepos = new DomainObjectDtoRepository( + 7000072, dtos, mockMdc, null, TestDirectoryFinder.LcmDirectories); + + m_dataMigrationManager.PerformMigration(dtoRepos, 7000073, new DummyProgressDlg()); + + // Assert the migration results + Assert.AreEqual(7000073, dtoRepos.CurrentModelVersion); + // Add specific assertions for your migration... + } + } +} +``` + +**API test**: For testing property access via the LCM API, inherit from `MemoryOnlyBackendProviderTestBase`. See the `writing-tests` skill. + +## Checklist + +- [ ] Property added to `MasterLCModel.xml` with correct `num`, `id`, `sig`, and (if relational) `card` +- [ ] `version` attribute incremented on `` +- [ ] Change history comment added at the top of the list +- [ ] FLEx Bridge metadata cache updated to match the new model number +- [ ] Migration class created OR `m_bumpNumberOnlyMigration` used +- [ ] Migration registered in `LcmDataMigrationManager` constructor +- [ ] If adding a C# value type: migration writes explicit defaults to all existing instances +- [ ] Build succeeds (`dotnet build -m:1 --configuration Release`) +- [ ] Migration test written +- [ ] Generated files NOT manually edited diff --git a/.claude/skills/adding-a-virtual-property/SKILL.md b/.claude/skills/adding-a-virtual-property/SKILL.md new file mode 100644 index 00000000..deed9ac3 --- /dev/null +++ b/.claude/skills/adding-a-virtual-property/SKILL.md @@ -0,0 +1,162 @@ +--- +name: adding-a-virtual-property +description: Add a computed or derived virtual property to an LCM class using the VirtualProperty attribute. No model version change, no migration, and no XML editing required. Use when the user asks to add a computed, derived, or virtual property that is not persisted. +--- + +# Adding a Virtual Property + +Virtual properties are computed/derived properties that are NOT persisted in the data store. They are discovered automatically via reflection from the `[VirtualProperty]` attribute. No model version change, no migration, and no XML editing is required. + +Use this when you need a property that: +- Computes a value from other persisted data +- Provides a back-reference (e.g., "all senses that reference this semantic domain") +- Exposes a convenience accessor + +If the property needs to be persisted, use the `adding-a-property` skill instead. + +## Steps + +### 1. Choose the Target File + +Virtual properties are added to partial class definitions in `src/SIL.LCModel/DomainImpl/`. Find the appropriate `Overrides*.cs` file: + +| File | Classes | +|------|---------| +| `OverridesLing_Lex.cs` | LexDb, LexEntry, LexSense, LexEntryRef, LexExampleSentence, etc. | +| `OverridesCellar.cs` | CmObject, CmPossibility, CmSemanticDomain, StText, StPara, etc. | +| `OverridesLing_Wfi.cs` | WfiWordform, WfiAnalysis, WfiGloss, WfiMorphBundle | +| `OverridesLing_MoClasses.cs` | MoForm, MoStemAllomorph, MoAffixAllomorph, MoMorphSynAnalysis, etc. | +| `OverridesLangProj.cs` | LangProject | +| `OverridesLing_Disc.cs` | DsConstChart, ConstChartRow, etc. | +| `OverridesNotebk.cs` | RnGenericRec | + +If the class doesn't have a partial class in any of these files yet, add a new `partial class` block to the appropriate file. + +### 2. Add the Property + +Add a public property with the `[VirtualProperty]` attribute inside the partial class. + +**Required imports:** +```csharp +using SIL.LCModel.Core.Cellar; // CellarPropertyType +using SIL.LCModel.Infrastructure; // VirtualPropertyAttribute +``` + +### 3. Choose the Right Pattern + +**Simple value type** (Integer, Boolean): +```csharp +[VirtualProperty(CellarPropertyType.Boolean)] +public bool IsSpecialCase +{ + get { return /* computed boolean expression */; } +} +``` + +**Reference collection** (back-references or computed lists): +```csharp +[VirtualProperty(CellarPropertyType.ReferenceCollection, "LexSense")] +public IEnumerable RelatedSenses +{ + get + { + // Compute and return the collection + return Services.GetInstance() + .AllInstances() + .Where(s => /* filter condition */); + } +} +``` + +The second parameter to `VirtualProperty` is the **signature** -- the unqualified class name of the target type. Required for all object-type properties (Reference*, Owning*). + +**Reference sequence** (ordered list): +```csharp +[VirtualProperty(CellarPropertyType.ReferenceSequence, "LexEntry")] +public IEnumerable OrderedEntries +{ + get { return /* computed ordered sequence */; } +} +``` + +**MultiUnicode** (computed multi-writing-system string): +```csharp +[VirtualProperty(CellarPropertyType.MultiUnicode)] +public IMultiAccessorBase ComputedTitle +{ + get + { + if (m_titleFlid == 0) + m_titleFlid = Cache.MetaDataCache.GetFieldId("ClassName", "ComputedTitle", false); + return new VirtualStringAccessor(this, m_titleFlid, ComputedTitleForWs); + } +} +private int m_titleFlid; + +private ITsString ComputedTitleForWs(int ws) +{ + // Return a TsString for the given writing system + return TsStringUtils.MakeString("computed value", ws); +} +``` + +**Reference atomic** (single computed reference): +```csharp +[VirtualProperty(CellarPropertyType.ReferenceAtomic, "CmPossibility")] +public ICmPossibility ComputedCategory +{ + get { return /* single object or null */; } +} +``` + +### 4. Property Registration + +No registration is needed. The `LcmMetaDataCache` automatically discovers properties with `[VirtualProperty]` via reflection during initialization. + +FLIDs are auto-assigned from 20,000,001 upward, in the order the attributes are constructed, with a ceiling of 30,000,000. Because the number depends on construction order, never persist a virtual FLID or hard-code one -- look it up by name through `MetaDataCache.GetFieldId`. + +### 5. Accessing Virtual Properties + +**From C# code** -- use the property directly: +```csharp +var senses = semanticDomain.ReferringSenses; +``` + +**From the SilDataAccess layer** (for views/UI integration): +```csharp +int flid = cache.MetaDataCache.GetFieldId("ClassName", "PropertyName", false); +var value = cache.DomainDataByFlid.get_Prop(obj.Hvo, flid); +``` + +### 6. Optional: Expose on the Interface + +If the virtual property should be accessible via the public interface (e.g., `ILexEntry`), add it to the hand-written partial interface. The generated interfaces are partial, so you can extend them: + +Partial interface extensions live in `src/SIL.LCModel/InterfaceAdditions.cs`, which already +carries about eighty of them. Add yours there rather than starting a new file. + +```csharp +// In src/SIL.LCModel/InterfaceAdditions.cs +namespace SIL.LCModel +{ + public partial interface ILexEntry + { + IEnumerable ComputedProperty { get; } + } +} +``` + +### 7. Write Tests + +Test virtual properties using `MemoryOnlyBackendProviderTestBase`. See the `writing-tests` skill. + +## Checklist + +- [ ] Property added to the correct partial class in `DomainImpl/Overrides*.cs` +- [ ] `[VirtualProperty]` attribute applied with correct `CellarPropertyType` +- [ ] Signature parameter provided for object-type properties +- [ ] Property is public and read-only (getter only) +- [ ] Interface extended if the property needs to be part of the public API +- [ ] No changes to `MasterLCModel.xml` (virtual properties are not persisted) +- [ ] No data migration needed +- [ ] Tests written diff --git a/.claude/skills/writing-a-data-migration/SKILL.md b/.claude/skills/writing-a-data-migration/SKILL.md new file mode 100644 index 00000000..233f4788 --- /dev/null +++ b/.claude/skills/writing-a-data-migration/SKILL.md @@ -0,0 +1,204 @@ +--- +name: writing-a-data-migration +description: Write a data migration class that transforms existing persisted XML data when the LCM model version changes. Covers migration structure, common operations (find, modify, create, remove objects), registration, version bumping, and tests. Use when the user asks to write a data migration, bump the model version, or transform existing persisted data. +--- + +# Writing a Data Migration + +Data migrations transform existing persisted data when the model version changes. They operate on raw XML via `DomainObjectDTO` objects — live `CmObject` instances are NOT available during migration. + +**Where things live:** Migration classes go in `src/SIL.LCModel/DomainServices/DataMigration/`. Each implements `IDataMigration` with a single `PerformMigration(IDomainObjectDTORepository)` method, uses `XElement.Parse()` for XML manipulation, and must call `DataMigrationServices.CheckVersionNumber` first and `DataMigrationServices.IncrementVersionNumber` last. Migrations are registered in `LcmDataMigrationManager`'s constructor (dictionary of version number to migration instance). The repository tracks changes in three sets: **newbies** (created), **dirtballs** (modified), **goners** (deleted). + +## When a Migration is Needed + +- Adding a C# value-type property (int, bool, GenDate, DateTime) that needs explicit defaults +- Removing a property or class (must clean up existing XML) +- Renaming a property or class +- Changing a property's type or cardinality +- Restructuring ownership or references +- Any change that requires existing persisted data to be transformed + +If the model change is purely additive (new optional reference/string property with no data to transform), you can use `m_bumpNumberOnlyMigration` in the manager instead. See step 4 in the `adding-a-property` skill. + +## Steps + +### 1. Determine the Next Version Number + +Check `src/SIL.LCModel/MasterLCModel.xml` for the current version in ``. Your migration file number is the next integer. + +### 2. Create the Migration Class + +The example below uses 7000072 -> 7000073. Substitute the numbers you read in step 1: the new file and class take the next version, and `CheckVersionNumber` takes the current one. + +Create: `src/SIL.LCModel/DomainServices/DataMigration/DataMigration7000073.cs` + +```csharp +// Copyright (c) YYYY SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System.Xml.Linq; + +namespace SIL.LCModel.DomainServices.DataMigration +{ + internal class DataMigration7000073 : IDataMigration + { + /// + /// Brief description of what this migration does. + /// + public void PerformMigration(IDomainObjectDTORepository repoDto) + { + DataMigrationServices.CheckVersionNumber(repoDto, 7000072); + + // --- Your migration logic here --- + + DataMigrationServices.IncrementVersionNumber(repoDto); + } + } +} +``` + +**Mandatory structure:** +1. First line: `DataMigrationServices.CheckVersionNumber(repoDto, previousVersion)` +2. Middle: your data transformation +3. Last line: `DataMigrationServices.IncrementVersionNumber(repoDto)` + +### 3. Common Migration Operations + +**Finding objects:** +```csharp +// All instances of a class (including subclasses) +var entries = repoDto.AllInstancesWithSubclasses("LexEntry"); + +// All instances of exact class (no subclasses) +var entries = repoDto.AllInstancesSansSubclasses("LexEntry"); + +// Single object by GUID +var dto = repoDto.GetDTO("guid-string-here"); + +// Owner of an object +var ownerDto = repoDto.GetOwningDTO(dto); + +// Directly owned children +var children = repoDto.GetDirectlyOwnedDTOs(dto.Guid); +``` + +**Modifying XML:** +```csharp +var element = XElement.Parse(dto.Xml); + +// Add an element +element.Add(new XElement("NewProperty", new XAttribute("val", "False"))); + +// Remove an element +element.Element("OldProperty")?.Remove(); + +// Add an objsur (owning reference) +var container = new XElement("OwnedThings"); +container.Add(new XElement("objsur", + new XAttribute("guid", targetGuid), + new XAttribute("t", "o"))); // "o" for owning, "r" for reference +element.Add(container); + +// Save changes +DataMigrationServices.UpdateDTO(repoDto, dto, element.ToString()); +``` + +**Creating new objects:** +```csharp +var newGuid = Guid.NewGuid().ToString().ToLowerInvariant(); +var sb = new StringBuilder(); +sb.AppendFormat("", newGuid, ownerGuid); +sb.Append(""); +sb.Append("value"); +sb.Append(""); +sb.Append(""); +repoDto.Add(new DomainObjectDTO(newGuid, "ClassName", sb.ToString())); +``` + +**Removing objects:** +```csharp +// Remove object, its owned children, and clean up owner's objsur +DataMigrationServices.RemoveIncludingOwnedObjects(repoDto, dto, removeFromOwner: true); +``` + +**Creating possibility lists** (use the helper): +```csharp +DataMigrationServices.CreatePossibilityList(repoDto, listGuid, ownerGuid, + new[] { Tuple.Create("en", "Abbr", "List Name") }, + DateTime.Now, WritingSystemServices.kwsAnals); +``` + +### 4. Register the Migration + +File: `src/SIL.LCModel/DomainServices/DataMigration/LcmDataMigrationManager.cs` + +Add at the end of the constructor's registration block: + +```csharp +m_individualMigrations.Add(7000073, new DataMigration7000073()); +``` + +### 5. Update MasterLCModel.xml Version + +File: `src/SIL.LCModel/MasterLCModel.xml` + +Update the version attribute and add a change history entry: +```xml + +``` + +### 6. Write Tests + +Create: `tests/SIL.LCModel.Tests/DomainServices/DataMigration/DataMigration7000073Tests.cs` + +```csharp +using System.Xml.Linq; +using NUnit.Framework; + +namespace SIL.LCModel.DomainServices.DataMigration +{ + [TestFixture] + public class DataMigration7000073Tests : DataMigrationTestsBase + { + [Test] + public void DataMigration7000073Test() + { + var dtos = DataMigrationTestServices.ParseProjectFile("DataMigration7000073.xml"); + var mockMdc = new MockMDCForDataMigration(); + IDomainObjectDTORepository dtoRepos = new DomainObjectDtoRepository( + 7000072, dtos, mockMdc, null, TestDirectoryFinder.LcmDirectories); + + m_dataMigrationManager.PerformMigration(dtoRepos, 7000073, new DummyProgressDlg()); + + Assert.AreEqual(7000073, dtoRepos.CurrentModelVersion); + // Add assertions verifying data was transformed correctly + } + } +} +``` + +**Test data file**: Create a minimal `.xml` file with sample `` elements in `tests/SIL.LCModel.Tests/TestData/`, where `DataMigrationTestServices.ParseProjectFile()` looks for it. Look at existing test data files like `DataMigration7000072.xml` for the expected format. + +## Key Rules + +- Migrations operate on raw XML strings via `DomainObjectDTO`. You cannot use `ICmObject` or any live LCM API. +- Use `XElement.Parse()` / `.ToString()` for XML manipulation. Do not use string replacement on XML. +- Always call `CheckVersionNumber` first and `IncrementVersionNumber` last. +- Handle null checks: optional elements may not exist in all objects. +- The repository tracks changes automatically. `UpdateDTO` marks as modified. `Add` marks as new. `Remove` marks as deleted. +- Use `AllInstancesWithSubclasses` when the property could be on subclasses too. +- GUIDs in the data are lowercase. Use `.ToLowerInvariant()` when comparing. +- For large migrations, organize logic into private helper methods (see `DataMigration7000065.cs` for this pattern). + +## Checklist + +- [ ] Migration class created with correct version number +- [ ] `CheckVersionNumber` called with previous version (N-1) +- [ ] `IncrementVersionNumber` called at the end +- [ ] Migration registered in `LcmDataMigrationManager` constructor +- [ ] `MasterLCModel.xml` version attribute updated +- [ ] Change history entry added to `MasterLCModel.xml` +- [ ] Test class created extending `DataMigrationTestsBase` +- [ ] Test data XML file created +- [ ] Build succeeds diff --git a/.claude/skills/writing-tests/SKILL.md b/.claude/skills/writing-tests/SKILL.md new file mode 100644 index 00000000..844b9f5a --- /dev/null +++ b/.claude/skills/writing-tests/SKILL.md @@ -0,0 +1,197 @@ +--- +name: writing-tests +description: Write NUnit tests for LCM model classes, domain services, data migrations, or API behavior. Covers test base classes, UnitOfWork patterns, object creation, string properties, repositories, and custom fields. Use when the user asks to write tests for LCM model classes, domain services, data migrations, or API behavior. +--- + +# Writing Tests + +Tests in liblcm use NUnit. The test infrastructure provides base classes that set up an `LcmCache` with the appropriate backend provider. + +## Test Project Structure + +Tests live in `tests/`: +- `SIL.LCModel.Tests/` -- Main library tests (model, domain services, infrastructure) +- `SIL.LCModel.Core.Tests/` -- Core utility tests +- `SIL.LCModel.Utils.Tests/` -- Utility tests +- `SIL.LCModel.FixData.Tests/` -- FixData tests + +## Base Classes + +### MemoryOnlyBackendProviderTestBase + +**Use for**: Testing the LCM public API (properties, factories, repositories, domain services). + +Located in `tests/SIL.LCModel.Tests/LcmTestBase.cs`. + +This creates a fresh in-memory `LcmCache` with a blank language project per test fixture. No file I/O. This is the most common base class. + +```csharp +using NUnit.Framework; +using SIL.LCModel.Infrastructure; + +namespace SIL.LCModel.SomeArea +{ + [TestFixture] + public class MyFeatureTests : MemoryOnlyBackendProviderTestBase + { + [Test] + public void MyTest() + { + // Cache is available via the Cache property + var lp = Cache.LanguageProject; + + // All data changes must be in a UnitOfWork + UndoableUnitOfWorkHelper.Do("undo", "redo", m_actionHandler, () => + { + // Create objects via factories + var entry = Cache.ServiceLocator.GetInstance().Create(); + + // Set properties + var ws = Cache.DefaultVernWs; + entry.CitationForm.VernacularDefaultWritingSystem = + TsStringUtils.MakeString("test", ws); + + // Assert + Assert.IsNotNull(entry); + }); + } + } +} +``` + +Key points: +- `Cache` property gives you the `LcmCache` +- `m_actionHandler` is the `IActionHandler` for UnitOfWork operations +- Default writing systems: `Cache.DefaultAnalWs` (English), `Cache.DefaultVernWs` (French) +- Use `UndoableUnitOfWorkHelper.Do()` or `NonUndoableUnitOfWorkHelper.Do()` for data changes + +### MemoryOnlyBackendProviderRestoredForEachTestTestBase + +**Use for**: Tests that need a clean state for each test method (not just each fixture). + +Same as above but disposes and recreates the cache before each `[Test]`. + +### DataMigrationTestsBase + +**Use for**: Testing data migrations. + +Located in `tests/SIL.LCModel.Tests/DomainServices/DataMigration/DataMigrationTests.cs`. + +Provides `m_dataMigrationManager` (an `IDataMigrationManager` instance). + +The example below uses 7000072 -> 7000073. Substitute the versions your migration actually +moves between: the repository is created at the previous version and the migration runs to +the new one. + +```csharp +using System.Xml.Linq; +using NUnit.Framework; + +namespace SIL.LCModel.DomainServices.DataMigration +{ + [TestFixture] + public class DataMigration7000073Tests : DataMigrationTestsBase + { + [Test] + public void DataMigration7000073Test() + { + // 1. Parse test data + var dtos = DataMigrationTestServices.ParseProjectFile("DataMigration7000073.xml"); + + // 2. Create repository at the PREVIOUS version + var mockMdc = new MockMDCForDataMigration(); + IDomainObjectDTORepository dtoRepos = new DomainObjectDtoRepository( + 7000072, dtos, mockMdc, null, TestDirectoryFinder.LcmDirectories); + + // 3. Run the migration + m_dataMigrationManager.PerformMigration(dtoRepos, 7000073, new DummyProgressDlg()); + + // 4. Verify version + Assert.AreEqual(7000073, dtoRepos.CurrentModelVersion); + + // 5. Verify data transformations + var dto = dtoRepos.GetDTO("some-guid-from-test-data"); + var element = XElement.Parse(dto.Xml); + Assert.IsNotNull(element.Element("ExpectedNewElement")); + } + } +} +``` + +**Test data files**: Migration tests use XML files containing sample `` elements. These live in `tests/SIL.LCModel.Tests/TestData/` and are parsed by `DataMigrationTestServices.ParseProjectFile()`. Look at existing files like `DataMigration7000072.xml` for the format. The file should contain a minimal set of `` elements that exercise the migration logic. + +## Common Test Patterns + +### Creating Test Objects + +```csharp +UndoableUnitOfWorkHelper.Do("undo", "redo", m_actionHandler, () => +{ + // Factories are accessed via ServiceLocator + var entryFactory = Cache.ServiceLocator.GetInstance(); + var senseFactory = Cache.ServiceLocator.GetInstance(); + + var entry = entryFactory.Create(); + var sense = senseFactory.Create(); + entry.SensesOS.Add(sense); +}); +``` + +### Setting String Properties + +```csharp +int vernWs = Cache.DefaultVernWs; +int analWs = Cache.DefaultAnalWs; + +// MultiUnicode +entry.CitationForm.set_String(vernWs, TsStringUtils.MakeString("word", vernWs)); + +// MultiString +sense.Definition.set_String(analWs, TsStringUtils.MakeString("a definition", analWs)); +``` + +### Accessing Repositories + +```csharp +var entryRepo = Cache.ServiceLocator.GetInstance(); +var allEntries = entryRepo.AllInstances(); +var count = entryRepo.Count; +``` + +### Testing with Custom Fields + +`CustomFieldForTest` is a protected nested class on `LcmTestBase`, so it is only reachable +from a test class that derives from one of the base classes above. + +```csharp +using (var customField = new CustomFieldForTest( + Cache, "My Field", "MyField", + LexEntryTags.kClassId, + CellarPropertyType.MultiUnicode, + Guid.Empty)) +{ + // customField.Flid gives you the field ID + // Test using the custom field... +} +// Custom field is automatically removed on Dispose +``` + +## Running Tests + +``` +dotnet test --no-restore --no-build -p:ParallelizeAssembly=false --configuration Release +``` + +Or run individual test classes/methods from your IDE. + +Tests must NOT run in parallel (`ParallelizeAssembly=false`) due to shared state in the ICU and writing system subsystems. + +## Checklist + +- [ ] Test class inherits from the appropriate base class +- [ ] `[TestFixture]` attribute on the class +- [ ] `[Test]` attribute on test methods +- [ ] All data changes wrapped in `UndoableUnitOfWorkHelper.Do()` or `NonUndoableUnitOfWorkHelper.Do()` +- [ ] Test assertions verify the expected behavior +- [ ] For migration tests: test data XML file created, repository initialized at previous version +- [ ] Tests pass, after a build: `dotnet test --no-restore --no-build -p:ParallelizeAssembly=false --configuration Release`