refactor(dedup): final pass — clear the last 4 src/ duplicate blocks #22

2026-05-27T17:28:50-05:00

bermudalamb commented

2026-05-27 17:28:50 -05:00

Summary

Closes out the remaining 4 duplicated blocks Sonar was reporting on src/ after PR #21. Two commits, both pure refactor with all 1180 tests staying green.

Commit	Block	Approach
`089d4f6`	CTE WITH-clause (Snowflake.QueryBreakdown ≡ SqlServer.QueryBreakdown, 24-line mid-method copy)	Hoist into `protected virtual void AppendWithClauseSection(StringBuilder, string withClauseIndent, string queryBodyIndent)` on SqlServer.QueryBreakdown. SqlServer passes 5/10-space indents; Snowflake passes 4/8. Standardizes on "WITH" + conditional " RECURSIVE" form (drops a trailing space in non-recursive SqlServer output)
`4038b3d`	Markdown CollectionGenerator pair (SqlServer ≡ PostgreSql, 82 lines)	Adapter pattern in `Markdown.Common`: new internal `ICollectionMarkdownData` interface + `CollectionMarkdownGenerator` template. Per-dialect wrappers shrink to a `Format` static + one-line forwarders + a private sealed `Adapter` nested class that handles the dialect-specific extraction and the `ParameterUsageReport → ParameterUsageRow` mapping that was previously copy-pasted three times

Notes

Public API unchanged — existing Markdown.{SqlServer,PostgreSql}.QueryBreakdownCollectionGenerator.GenerateX(...) calls work identically; their bodies just delegate.
The three dialect-specific ParameterUsageReport classes are deliberately not unified. Their ToString() overrides differ meaningfully per dialect (T-SQL @name vs PG :name/$n vs Snowflake with a no-queries branch), and unifying would be a separate API discussion. The adapter pattern dedupes the consumption of these types in Markdown without touching the types themselves.
Snowflake's Markdown.Snowflake.QueryBreakdownCollectionGenerator is intentionally left alone — Sonar didn't flag it (the GenerateSnowflakeFeaturesAnalysis + feature-aware QueryCompositionReport callback make it structurally distinct enough). A consistency follow-up could put it on the same adapter pattern without behavior change.

Test plan

dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release — 0 errors after each commit
dotnet test ... -c Release --no-build — 1180 passed, 0 failed
Gitea Actions sonarqube.yml scans this PR
Re-query /api/measures/component?metricKeys=duplicated_blocks — should drop from 4 to 0

## Summary Closes out the remaining 4 duplicated blocks Sonar was reporting on `src/` after PR #21. Two commits, both pure refactor with all 1180 tests staying green. | Commit | Block | Approach | |---|---|---| | `089d4f6` | CTE WITH-clause (Snowflake.QueryBreakdown ≡ SqlServer.QueryBreakdown, 24-line mid-method copy) | Hoist into `protected virtual void AppendWithClauseSection(StringBuilder, string withClauseIndent, string queryBodyIndent)` on SqlServer.QueryBreakdown. SqlServer passes 5/10-space indents; Snowflake passes 4/8. Standardizes on "WITH" + conditional " RECURSIVE" form (drops a trailing space in non-recursive SqlServer output) | | `4038b3d` | Markdown CollectionGenerator pair (SqlServer ≡ PostgreSql, 82 lines) | Adapter pattern in `Markdown.Common`: new internal `ICollectionMarkdownData` interface + `CollectionMarkdownGenerator` template. Per-dialect wrappers shrink to a `Format` static + one-line forwarders + a private sealed `Adapter` nested class that handles the dialect-specific extraction and the `ParameterUsageReport → ParameterUsageRow` mapping that was previously copy-pasted three times | ## Notes - Public API unchanged — existing `Markdown.{SqlServer,PostgreSql}.QueryBreakdownCollectionGenerator.GenerateX(...)` calls work identically; their bodies just delegate. - The three dialect-specific `ParameterUsageReport` classes are deliberately **not** unified. Their `ToString()` overrides differ meaningfully per dialect (T-SQL `@name` vs PG `:name`/`$n` vs Snowflake with a no-queries branch), and unifying would be a separate API discussion. The adapter pattern dedupes the *consumption* of these types in Markdown without touching the types themselves. - Snowflake's `Markdown.Snowflake.QueryBreakdownCollectionGenerator` is intentionally left alone — Sonar didn't flag it (the `GenerateSnowflakeFeaturesAnalysis` + feature-aware `QueryCompositionReport` callback make it structurally distinct enough). A consistency follow-up could put it on the same adapter pattern without behavior change. ## Test plan - [x] `dotnet build Strata.SqlTools.QueryBreakdown.sln -c Release` — 0 errors after each commit - [x] `dotnet test ... -c Release --no-build` — 1180 passed, 0 failed - [ ] Gitea Actions sonarqube.yml scans this PR - [ ] Re-query `/api/measures/component?metricKeys=duplicated_blocks` — should drop from 4 to 0

bermudalamb added 2 commits 2026-05-27 17:28:51 -05:00

refactor(dedup): extract AppendWithClauseSection for the WITH/CTE block 089d4f6000

`SqlServer.QueryBreakdown.GetSqlBreakdown` and
`Snowflake.QueryBreakdown.GetSql` each carried a 24-line copy of the
same CTE-rendering loop ("WITH" keyword, optional RECURSIVE, per-clause
header, anchor/UNION ALL/recursive query, closing parens). The two
copies differed only by indent (5/10 spaces vs 4/8) and a trailing
space after the keyword.

Hoist the loop into `protected virtual void AppendWithClauseSection(
StringBuilder, string withClauseIndent, string queryBodyIndent)` on
`SqlServer.QueryBreakdown`. Each caller invokes it with its dialect's
preferred indents; Snowflake's mid-method copy is deleted entirely.

Standardizes on the no-trailing-space "WITH" form (Snowflake's) — was
"WITH " (trailing space) in the SqlServer original. Visible only as a
trailing space before the newline in non-recursive output, which no
tests assert on.

All 1180 tests stay green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

refactor(dedup): adapter-based Markdown CollectionGenerator dedup (the big one)

SonarQube Analysis / sonarqube (pull_request) Successful in 3m42s

Details

4038b3dab5

Final cluster Sonar was reporting: the 82-line copy-paste between
`Markdown.SqlServer.QueryBreakdownCollectionGenerator` and
`Markdown.PostgreSql.QueryBreakdownCollectionGenerator`. Both classes
existed because each dialect has a different concrete
`QueryBreakdownCollection` type with its own `ParameterUsageReport`
class — no shared base for the methods to operate on.

Resolves it with an adapter pattern in `Markdown.Common`:

- **`ICollectionMarkdownData`** (new, internal): dialect-neutral view
  exposing query count, parameter / column / table totals, queries-
  for-report list, and parameter-rows (already-mapped to the writer's
  `ParameterUsageRow` type).
- **`CollectionMarkdownGenerator`** (new, internal static): single
  template that takes the data + `MarkdownDialectFormat` and routes
  through `CollectionReportWriter`. The six `GenerateX` methods that
  were duplicated three times now live here once.
- **SqlServer / PostgreSql wrappers**: shrunk to a `Format` static, a
  thin one-line forwarder per public method, and a private sealed
  `Adapter : ICollectionMarkdownData` nested class that does the
  dialect-specific extraction (including the `ParameterUsageReport →
  ParameterUsageRow` mapping that was previously duplicated three
  times as `MapParameters`).

Public API unchanged — the existing `Markdown.SqlServer.QueryBreakdownCollectionGenerator.GenerateCollectionReport(collection, title)`
etc. continue to work as before; their bodies just delegate. The
three dialect-specific `ParameterUsageReport` classes are
deliberately *not* unified yet — their `ToString()` overrides differ
meaningfully per dialect and unifying would be a separate API
discussion.

Snowflake wrapper not touched in this commit — Sonar didn't flag it
(its `GenerateSnowflakeFeaturesAnalysis` and feature-aware
`QueryCompositionReport` callback make it structurally distinct).
Consistency follow-up could move it onto the same adapter pattern
without behavior change.

All 1180 tests stay green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

bermudalamb merged commit d70693a01d into main

2026-05-27 17:32:37 -05:00

bermudalamb deleted branch chore/sonarqube-src-dedup-final

2026-05-27 17:32:38 -05:00

bermudalamb referenced this issue from a commit

2026-05-27 17:47:14 -05:00

refactor(dedup): IQueryBreakdownCollectionView eliminates Markdown adapter classes

bermudalamb referenced this pull request

2026-05-27 17:48:06 -05:00

refactor(dedup): IQueryBreakdownCollectionView eliminates Markdown adapter classes #23

Sign in to join this conversation.