Files
sql-utilities/docs/superpowers/plans/2026-05-20-shared-query-project.md
T
Thom LambandClaude Opus 4.7 70c9a3b3dc docs: implementation plan for Strata.SqlTools.Query extraction
Task-by-task plan to scaffold the shared project, move the 19 identical
ExpressionFactory/Query files, rewire dialect references, and clear the 3
remaining new-code SonarQube smells (IDE0028 x2, NUnit2045).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-20 11:05:59 -05:00

422 lines
18 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Extract `Strata.SqlTools.Query` shared project + clear 3 new-code smells — Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Eliminate the Snowflake/SqlServer `ExpressionFactory/Query` duplication by extracting the byte-identical 19-file tree into a new `Strata.SqlTools.Query` project, and resolve the 3 remaining new-code SonarQube smells.
**Architecture:** A new standalone class library (`Strata.SqlTools.Query`, BCL-only, zero project references) holds one copy of the 19 query-config model types under the flat namespace `Strata.SqlTools.Query`. Both `Strata.SqlTools.Snowflake` and `Strata.SqlTools.SqlServer` reference it; their local copies are deleted. The existing ~819-test suite is the regression safety net.
**Tech Stack:** C# / .NET 8 / NUnit / SonarAnalyzer.CSharp 10.4.0 (via `Directory.Build.props`).
Implements `docs/superpowers/specs/2026-05-20-shared-query-project-design.md`.
**Working directory:** All paths relative to `C:\gitea\sql-utilities`. Branch `fix/Sonarqube-Tech-Debt` is checked out; `HEAD` is the spec commit `55ab737`.
---
## Why this is safe (facts established during planning)
- The 19 files in `src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/` and `src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/` are **byte-identical** except the `namespace` line.
- Those files reference **only each other and BCL types** (no `using` beyond `System.*`; no `SqlBreakdown`/`Rules` references) → the new project needs no project references.
- The **only** external consumers with an explicit `using …ExpressionFactory.Query;` are 3 files (all reference the SqlServer namespace):
- `src/Strata.SqlTools.SqlServer/ExpressionFactory/ExpressionFactory.cs`
- `tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionFactoryFilterTests.cs`
- `tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionTestsBase.cs`
- Other consumers reference the types **relatively** as `Query.X` (e.g. Snowflake `ExpressionFactory.cs`, whose namespace is `Strata.SqlTools.Snowflake.ExpressionFactory`). After the move these resolve automatically, because `Strata.SqlTools.Query` is reachable as `Query` from any `Strata.SqlTools.*` namespace.
- **No file references both dialect Query namespaces**, so collapsing the two type families into one cannot create ambiguous overloads.
---
## File map
**Created:**
- `src/Strata.SqlTools.Query/Strata.SqlTools.Query.csproj` — new BCL-only class library
- `src/Strata.SqlTools.Query/*.cs` — the 19 moved model files (flat namespace `Strata.SqlTools.Query`)
**Modified:**
- `Strata.SqlTools.QueryBreakdown.sln` — add the new project
- `src/Strata.SqlTools.SqlServer/Strata.SqlTools.SqlServer.csproj` — add `ProjectReference` to `Strata.SqlTools.Query`
- `src/Strata.SqlTools.Snowflake/Strata.SqlTools.Snowflake.csproj` — add `ProjectReference` to `Strata.SqlTools.Query`
- `src/Strata.SqlTools.SqlServer/ExpressionFactory/ExpressionFactory.cs` — update `using`
- `tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionFactoryFilterTests.cs` — update `using`
- `tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionTestsBase.cs` — update `using`
- `src/Strata.SqlTools.Query/CalculationFilterGroup.cs` — IDE0028 fix (`?? []`)
- `tests/Strata.SqlTools.PostgreSql.Tests/PostgreSql/CommandVisitorTests.cs` — NUnit2045 fix (`Assert.Multiple`)
**Deleted:**
- `src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/*.cs` (19 files)
- `src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/*.cs` (19 files — moved, not copied)
---
## Task 1: Scaffold the `Strata.SqlTools.Query` project
**Files:**
- Create: `src/Strata.SqlTools.Query/Strata.SqlTools.Query.csproj`
- Modify: `Strata.SqlTools.QueryBreakdown.sln`
- [ ] **Step 1: Record the baseline test result**
Run:
```powershell
dotnet test Strata.SqlTools.QueryBreakdown.sln -c Release --nologo
```
Expected: build succeeds; full suite green. **Write down the total passed count** printed at the end — every later test step must match or exceed it (Task 3 leaves it unchanged; no tests are added or removed by this plan).
- [ ] **Step 2: Create the project directory and csproj**
Create `src/Strata.SqlTools.Query/Strata.SqlTools.Query.csproj` with this exact content (mirrors the dialect csprojs minus dialect-specific package refs):
```xml
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net8.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<LangVersion>latest</LangVersion>
<!-- NuGet Package Metadata -->
<PackageId>Strata.SqlTools.Query</PackageId>
<Version>1.0.0</Version>
<Authors>Strata Decision Technology</Authors>
<Company>Strata Decision Technology</Company>
<Product>Strata SQL Utilities - Query Model</Product>
<Description>Dialect-agnostic query configuration model (QueryConfig, filters, values, rows) shared by the Strata.SqlTools SQL Server and Snowflake dialect packages.</Description>
<PackageTags>sql;query-builder;query-config;database</PackageTags>
<PackageProjectUrl>https://github.com/stratadecision/sql-builder</PackageProjectUrl>
<RepositoryUrl>https://github.com/stratadecision/sql-builder</RepositoryUrl>
<RepositoryType>git</RepositoryType>
<PackageLicenseExpression>MIT</PackageLicenseExpression>
<PackageReadmeFile>README.md</PackageReadmeFile>
<PackageReleaseNotes>Initial release: shared query configuration model extracted from the dialect packages.</PackageReleaseNotes>
<Copyright>Copyright © Strata Decision Technology 2024-2026</Copyright>
<!-- Build Configuration -->
<GeneratePackageOnBuild>false</GeneratePackageOnBuild>
<IncludeSymbols>true</IncludeSymbols>
<SymbolPackageFormat>symbols.nupkg</SymbolPackageFormat>
<EmbedUntrackedSources>true</EmbedUntrackedSources>
<ContinuousIntegrationBuild Condition="'$(CI)' == 'true'">true</ContinuousIntegrationBuild>
<!-- Code Analysis -->
<EnableNETAnalyzers>true</EnableNETAnalyzers>
<AnalysisLevel>latest</AnalysisLevel>
<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
</PropertyGroup>
<ItemGroup>
<None Include="..\..\README.md" Pack="true" PackagePath="\" />
</ItemGroup>
</Project>
```
- [ ] **Step 3: Add the project to the solution**
Run:
```powershell
dotnet sln Strata.SqlTools.QueryBreakdown.sln add src/Strata.SqlTools.Query/Strata.SqlTools.Query.csproj
```
Expected: `Project ... added to the solution.`
- [ ] **Step 4: Build the (empty) project to confirm it is well-formed**
Run:
```powershell
dotnet build src/Strata.SqlTools.Query/Strata.SqlTools.Query.csproj -c Release --nologo
```
Expected: `Build succeeded.` (It compiles to an empty assembly — no .cs files yet.)
- [ ] **Step 5: Commit**
```powershell
git add src/Strata.SqlTools.Query/Strata.SqlTools.Query.csproj Strata.SqlTools.QueryBreakdown.sln
git commit -m "build(query): scaffold empty Strata.SqlTools.Query shared project
First step of extracting the duplicated ExpressionFactory/Query model
tree. Adds a BCL-only class library and registers it in the solution;
files are moved in the next commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>"
```
---
## Task 2: Move the 19 model files into the shared project and rewire references
This task is atomic: the move, namespace rename, reference wiring, duplicate deletion, and `using` fixes must all land together for the solution to compile. Do **not** split the commit.
**Files:**
- Move (via `git mv`): `src/Strata.SqlTools.SqlServer/ExpressionFactory/Query/*.cs``src/Strata.SqlTools.Query/`
- Delete: `src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/*.cs`
- Modify: both dialect csprojs, the 3 explicit-`using` files, and `src/Strata.SqlTools.Query/CalculationFilterGroup.cs`
- [ ] **Step 1: Move SqlServer's 19 files into the new project (preserving git history)**
Run:
```powershell
$src = "src/Strata.SqlTools.SqlServer/ExpressionFactory/Query"
$dst = "src/Strata.SqlTools.Query"
Get-ChildItem "$src/*.cs" | ForEach-Object { git mv $_.FullName "$dst/$($_.Name)" }
Get-ChildItem "$dst/*.cs" | Measure-Object | Select-Object -ExpandProperty Count
```
Expected: prints `19`. The SqlServer `ExpressionFactory/Query` folder is now empty on disk (git does not track empty folders).
- [ ] **Step 2: Rename the namespace in all 19 moved files**
Run:
```powershell
Get-ChildItem "src/Strata.SqlTools.Query/*.cs" | ForEach-Object {
$p = $_.FullName
(Get-Content $p) -replace 'namespace Strata\.SqlTools\.SqlServer\.ExpressionFactory\.Query;', 'namespace Strata.SqlTools.Query;' | Set-Content $p
}
Select-String -Path "src/Strata.SqlTools.Query/*.cs" -Pattern '^namespace ' | Select-Object -ExpandProperty Line | Sort-Object -Unique
```
Expected: the only line printed is `namespace Strata.SqlTools.Query;` (all 19 files now share the flat namespace).
- [ ] **Step 3: Delete Snowflake's duplicate 19 files**
Run:
```powershell
git rm src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/*.cs
```
Expected: `rm 'src/Strata.SqlTools.Snowflake/ExpressionFactory/Query/...'` ×19.
- [ ] **Step 4: Add the project reference to SqlServer**
In `src/Strata.SqlTools.SqlServer/Strata.SqlTools.SqlServer.csproj`, locate the `ItemGroup` containing the `SqlBreakdown` project reference:
```xml
<ItemGroup>
<ProjectReference Include="..\Strata.SqlTools.SqlBreakdown\Strata.SqlTools.SqlBreakdown.csproj" />
</ItemGroup>
```
Replace it with:
```xml
<ItemGroup>
<ProjectReference Include="..\Strata.SqlTools.SqlBreakdown\Strata.SqlTools.SqlBreakdown.csproj" />
<ProjectReference Include="..\Strata.SqlTools.Query\Strata.SqlTools.Query.csproj" />
</ItemGroup>
```
- [ ] **Step 5: Add the project reference to Snowflake**
In `src/Strata.SqlTools.Snowflake/Strata.SqlTools.Snowflake.csproj`, locate:
```xml
<ItemGroup>
<ProjectReference Include="..\Strata.SqlTools.SqlServer\Strata.SqlTools.SqlServer.csproj" />
</ItemGroup>
```
Replace it with:
```xml
<ItemGroup>
<ProjectReference Include="..\Strata.SqlTools.SqlServer\Strata.SqlTools.SqlServer.csproj" />
<ProjectReference Include="..\Strata.SqlTools.Query\Strata.SqlTools.Query.csproj" />
</ItemGroup>
```
- [ ] **Step 6: Fix the 3 explicit `using` directives**
In each of these files, change the line
`using Strata.SqlTools.SqlServer.ExpressionFactory.Query;`
to
`using Strata.SqlTools.Query;`
- `src/Strata.SqlTools.SqlServer/ExpressionFactory/ExpressionFactory.cs`
- `tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionFactoryFilterTests.cs`
- `tests/Strata.SqlTools.SqlBreakdown.Tests/ExpressionTests/ExpressionTestsBase.cs`
Verify:
```powershell
Get-ChildItem src,tests -Recurse -Filter *.cs | Select-String -Pattern 'SqlServer\.ExpressionFactory\.Query'
```
Expected: **no matches** (no source file references the old namespace anymore).
- [ ] **Step 7: Fix IDE0028 in the moved `CalculationFilterGroup.cs`**
In `src/Strata.SqlTools.Query/CalculationFilterGroup.cs`, change the `GetValidFilters` body:
Before:
```csharp
public IEnumerable<CalculationFilter> GetValidFilters()
{
return Filters?.Where(x => x.IsValid()).ToList() ?? new List<CalculationFilter>();
}
```
After:
```csharp
public IEnumerable<CalculationFilter> GetValidFilters()
{
return Filters?.Where(x => x.IsValid()).ToList() ?? [];
}
```
(This single edit resolves both original IDE0028 issues, which were the two now-merged copies.)
- [ ] **Step 8: Build the full solution**
Run:
```powershell
dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release --nologo
```
Expected: `Build succeeded.`
**If you see `CS0246: The type or namespace name 'X' could not be found`** in a file not listed above: that file referenced a Query type by bare name without resolving it relatively. Fix it by adding a file-scoped `using Strata.SqlTools.Query;` to that specific file (do **not** add a project-wide global using — generic names like `Filter`/`Value`/`Field`/`Row` could become ambiguous). Re-run the build until clean.
- [ ] **Step 9: Run the full test suite**
Run:
```powershell
dotnet test Strata.SqlTools.QueryBreakdown.sln -c Release --no-build --nologo
```
Expected: all tests pass; total count equals the baseline recorded in Task 1 Step 1 (no tests added or removed).
- [ ] **Step 10: Commit**
```powershell
git add -A
git commit -m "refactor(query): extract shared Strata.SqlTools.Query model project
Moves the byte-identical 19-file ExpressionFactory/Query tree (duplicated
across Snowflake and SqlServer) into the new Strata.SqlTools.Query project
under the flat namespace Strata.SqlTools.Query. Both dialect projects now
reference the shared project; the Snowflake copies are deleted.
Also folds in the IDE0028 fix on CalculationFilterGroup.GetValidFilters
(collection expression `[]`), which resolves both new-code IDE0028 smells
in one place now that there is a single copy.
Eliminates the 63 new duplicate lines flagged on the PR and removes the
largest contributor to the project's 11.1% duplication density. No
behavioral change: the moved types are identical to the originals.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>"
```
---
## Task 3: Fix NUnit2045 in `CommandVisitorTests`
**Files:**
- Modify: `tests/Strata.SqlTools.PostgreSql.Tests/PostgreSql/CommandVisitorTests.cs:18-20`
**Background:** SonarQube flags `NUnit2045` because the three independent `Assert.That` calls run sequentially — a failure in the first hides the others. Wrapping them in `Assert.Multiple` reports all three together. The `Assert.That(method, Is.Not.Null, …)` guard in the helper method stays as-is: it gates a subsequent reflection `Invoke`, so it must short-circuit and must **not** be inside a multiple block.
- [ ] **Step 1: Wrap the three assertions in `Assert.Multiple`**
In `tests/Strata.SqlTools.PostgreSql.Tests/PostgreSql/CommandVisitorTests.cs`, replace lines 18-20:
Before:
```csharp
Assert.That(param1a, Is.EqualTo("$1"), "first visitor's first parameter should be $1");
Assert.That(param1b, Is.EqualTo("$2"), "first visitor's second parameter should be $2");
Assert.That(param2a, Is.EqualTo("$1"), "second visitor must start at $1, not inherit visitor1's counter");
```
After:
```csharp
Assert.Multiple(() =>
{
Assert.That(param1a, Is.EqualTo("$1"), "first visitor's first parameter should be $1");
Assert.That(param1b, Is.EqualTo("$2"), "first visitor's second parameter should be $2");
Assert.That(param2a, Is.EqualTo("$1"), "second visitor must start at $1, not inherit visitor1's counter");
});
```
- [ ] **Step 2: Run the affected test**
Run:
```powershell
dotnet test tests/Strata.SqlTools.PostgreSql.Tests/Strata.SqlTools.PostgreSql.Tests.csproj -c Release --filter "FullyQualifiedName~CommandVisitorTests" --nologo
```
Expected: PASS (1 test). Behavior is unchanged — only the assertion grouping changed.
- [ ] **Step 3: Commit**
```powershell
git add tests/Strata.SqlTools.PostgreSql.Tests/PostgreSql/CommandVisitorTests.cs
git commit -m "test(pgsql): wrap CommandVisitor asserts in Assert.Multiple
Resolves SonarQube NUnit2045. The three independent parameter-index
assertions now report together instead of short-circuiting on the first
failure. The Is.Not.Null guard in the reflection helper stays outside the
multiple block because it gates the subsequent Invoke.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>"
```
---
## Final verification
- [ ] **Step 1: Clean working tree, full build + test**
Run:
```powershell
git status --short
dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release --nologo
dotnet test Strata.SqlTools.QueryBreakdown.sln -c Release --no-build --nologo
```
Expected: empty working tree; clean build; full suite green at the baseline count from Task 1 Step 1.
- [ ] **Step 2: Confirm no duplicate model files remain**
Run:
```powershell
Get-ChildItem -Recurse -Filter CalculationFilterGroup.cs src | Select-Object FullName
```
Expected: exactly one path — `src/Strata.SqlTools.Query/CalculationFilterGroup.cs`. (No dialect copies.)
- [ ] **Step 3: Push a fresh SonarQube analysis (optional but recommended)**
Requires `$env:SONAR_TOKEN`. Run:
```powershell
./scan-sonar.ps1
```
Then confirm on `https://snrqbe.bermudalamb.synology.me` (project `sql-utilities`):
- New-code period shows **0** code smells (the two IDE0028 and the NUnit2045 are gone).
- New-code **duplicated lines = 0**.
- Project-wide `duplicated_lines_density` has dropped from 11.1%.
Ad-hoc check (token as Basic-auth username; see project memory):
```powershell
$b64 = [Convert]::ToBase64String([Text.Encoding]::ASCII.GetBytes("$($env:SONARQUBE_TOKEN):"))
$h = @{ Authorization = "Basic $b64" }
Invoke-RestMethod -Uri "$($env:SONARQUBE_URL)/api/issues/search?componentKeys=sql-utilities&inNewCodePeriod=true&resolved=false" -Headers $h |
Select-Object -ExpandProperty total
```
Expected: `0`.
- [ ] **Step 4: Merge decision**
Do **not** push to the Gitea remote automatically (user works locally). Report the commit list (`git log --oneline main..HEAD`) and let the user choose merge / PR / hold.
---
## Out of scope (do not do in this plan)
- The other ~798 project-wide code smells (S3776 cognitive complexity, CA/IDE info-level, etc.).
- Extracting non-identical dialect code (visitors, breakdowns, statement parsers).
- Backward-compat `[TypeForwardedTo]` shims (packages are unpublished; revisit only if external consumers appear).
- Pushing the branch to remote.