diff --git a/internal/db/repository/repository_integration_test.go b/internal/db/repository/repository_integration_test.go index bdd6046..1de34ec 100644 --- a/internal/db/repository/repository_integration_test.go +++ b/internal/db/repository/repository_integration_test.go @@ -22,20 +22,9 @@ import ( var testPool *pgxpool.Pool // TestMain starts a single Postgres container for the whole package and seeds -// two fixture tables: -// -// - tabula.germany: quoted, mixed-case columns ("Code_BuildingVariant", ...) - -// the shape TableConstructor actually produces in production, and what -// TabulaRepository's quoted queries expect. -// - public.building_fixture: unquoted lowercase columns - what -// BuildingRepository.GetByBuildingCode's literal (unquoted) query expects. -// NOTE: these two shapes don't match. BuildingRepository is not wired up -// to any handler in production (see internal/service.NewIgnisServiceWithDB, -// which nothing calls), so this mismatch has never surfaced as a bug - -// but querying a real TableConstructor-created table with -// GetByBuildingCode would return "no data found" today. This fixture -// locks in BuildingRepository's current, documented behaviour rather than -// silently changing it. +// tabula.germany with quoted, mixed-case columns ("Code_BuildingVariant", ...) +// - the shape TableConstructor actually produces in production, and what +// TabulaRepository's quoted queries expect. func TestMain(m *testing.M) { ctx := context.Background() @@ -117,12 +106,6 @@ func seedFixtures(ctx context.Context, pool *pgxpool.Pool) error { ('DE.N.SFH.01.Gen', 75.5, 185, 123.45), ('DE.N.SFH.01.ReEx', 75.5, 185, 98.70), ('DE.N.MFH.01.Gen', 200.0, 185, 88.10)`, - `CREATE TABLE building_fixture ( - id SERIAL PRIMARY KEY, - code_buildingvariant VARCHAR, - a_roof_1 REAL - )`, - `INSERT INTO building_fixture (code_buildingvariant, a_roof_1) VALUES ('DE.N.SFH.01.Gen', 75.5)`, } for _, stmt := range statements { if _, err := pool.Exec(ctx, stmt); err != nil { @@ -219,57 +202,3 @@ func TestTabulaRepository_GetVariant_queryError(t *testing.T) { t.Error("a query error against a missing table should not be reported as ErrVariantNotFound") } } - -// --- BuildingRepository --- -// See the TestMain doc comment: this repository's unquoted query only matches -// a table created with unquoted (hence lowercased) column names, which is -// not what TableConstructor actually produces - building_fixture mirrors -// BuildingRepository's own literal expectation, not production reality. - -// TestBuildingRepository_GetByBuildingCode_success finds a row (the unquoted -// WHERE clause matches building_fixture's unquoted lowercase columns) but -// documents a second, compounding mismatch: populateStructFromMap looks up -// scanned values by the mixed-case JSON tag ("A_Roof_1"), while Postgres -// returns unquoted-created columns lowercased ("a_roof_1") in -// FieldDescriptions - so the map lookup misses and the field stays zero even -// on a "successful" (no error) call. Combined with the quoted-table mismatch -// above, GetByBuildingCode cannot correctly populate a struct from either -// table shape as currently written; this is presumably why it's unused in -// production (see TestMain's doc comment). -func TestBuildingRepository_GetByBuildingCode_success(t *testing.T) { - r := repository.NewBuildingRepository(testPool, "") - params, err := r.GetByBuildingCode(context.Background(), "building_fixture", "DE.N.SFH.01.Gen") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if params.BasicParameters.Envelope.A_Roof_1 != 0 { - t.Errorf("A_Roof_1 = %v, want 0 (case-mismatched column lookup silently fails to populate) - if this now populates, the bug has been fixed and this test should be updated to assert 75.5", params.BasicParameters.Envelope.A_Roof_1) - } -} - -func TestBuildingRepository_GetByBuildingCode_notFound(t *testing.T) { - r := repository.NewBuildingRepository(testPool, "") - _, err := r.GetByBuildingCode(context.Background(), "building_fixture", "DE.N.SFH.99.Gen") - if err == nil { - t.Fatal("expected error for unknown building code") - } -} - -func TestBuildingRepository_GetByBuildingCode_queryError(t *testing.T) { - r := repository.NewBuildingRepository(testPool, "") - _, err := r.GetByBuildingCode(context.Background(), "nonexistent_table", "DE.N.SFH.01.Gen") - if err == nil { - t.Fatal("expected error for nonexistent table") - } -} - -// mismatchAgainstProductionShape documents (rather than silently fixes) the -// case-sensitivity gap: BuildingRepository's query cannot find a row in a -// table shaped like TableConstructor's real output. -func TestBuildingRepository_GetByBuildingCode_mismatchAgainstProductionShape(t *testing.T) { - r := repository.NewBuildingRepository(testPool, "tabula") - _, err := r.GetByBuildingCode(context.Background(), "germany", "DE.N.SFH.01.Gen") - if err == nil { - t.Fatal("expected GetByBuildingCode's unquoted query to fail to find a row in a quoted-column production-shaped table - if this now passes, the case-sensitivity mismatch has been fixed and this test (and its doc comment) should be updated") - } -} diff --git a/internal/db/repository/building_repository.go b/internal/db/repository/struct_mapper.go similarity index 71% rename from internal/db/repository/building_repository.go rename to internal/db/repository/struct_mapper.go index 1ca65c0..6194956 100644 --- a/internal/db/repository/building_repository.go +++ b/internal/db/repository/struct_mapper.go @@ -1,61 +1,12 @@ package repository import ( - "context" "fmt" "reflect" "github.com/thd-spatial-ai/ignis/internal/models" - - "github.com/jackc/pgx/v5" - "github.com/jackc/pgx/v5/pgxpool" ) -// BuildingRepository handles database operations for building data. -type BuildingRepository struct { - pool *pgxpool.Pool - schema string -} - -// NewBuildingRepository creates a new building repository. -// schema is the PostgreSQL schema name (e.g. "tabula"); pass "" to use the search_path default. -func NewBuildingRepository(pool *pgxpool.Pool, schema string) *BuildingRepository { - return &BuildingRepository{pool: pool, schema: schema} -} - -// GetByBuildingCode retrieves a building by building variant code. -func (r *BuildingRepository) GetByBuildingCode(ctx context.Context, tableName string, buildingCode string) (*models.TabulaBuildingParameters, error) { - query := fmt.Sprintf(`SELECT * FROM %s WHERE code_buildingvariant = $1 LIMIT 1`, r.qualifyTable(tableName)) - - rows, err := r.pool.Query(ctx, query, buildingCode) - if err != nil { - return nil, fmt.Errorf("failed to query building data: %w", err) - } - defer rows.Close() - - if !rows.Next() { - return nil, fmt.Errorf("no data found for building code %s", buildingCode) - } - - dataMap, err := rowsToDataMap(rows) - if err != nil { - return nil, err - } - - // Initialize and populate TabulaBuildingParameters - tabulaData := initializeTabulaData() - populateStructFromMap(tabulaData, dataMap) - - return tabulaData, nil -} - -func (r *BuildingRepository) qualifyTable(tableName string) string { - if r.schema == "" { - return pgx.Identifier{tableName}.Sanitize() - } - return pgx.Identifier{r.schema, tableName}.Sanitize() -} - // initializeTabulaData creates a fully initialized TabulaBuildingParameters with all nested structs func initializeTabulaData() *models.TabulaBuildingParameters { data := &models.TabulaBuildingParameters{ diff --git a/internal/db/repository/building_repository_test.go b/internal/db/repository/struct_mapper_test.go similarity index 94% rename from internal/db/repository/building_repository_test.go rename to internal/db/repository/struct_mapper_test.go index 5f5c34f..2eb828d 100644 --- a/internal/db/repository/building_repository_test.go +++ b/internal/db/repository/struct_mapper_test.go @@ -5,21 +5,6 @@ import ( "testing" ) -func TestBuildingRepository_qualifyTable(t *testing.T) { - cases := []struct { - schema, table, want string - }{ - {"tabula", "germany", `"tabula"."germany"`}, - {"", "germany", `"germany"`}, - } - for _, tc := range cases { - r := NewBuildingRepository(nil, tc.schema) - if got := r.qualifyTable(tc.table); got != tc.want { - t.Errorf("qualifyTable(schema=%q, table=%q) = %q, want %q", tc.schema, tc.table, got, tc.want) - } - } -} - func TestInitializeTabulaData(t *testing.T) { data := initializeTabulaData() diff --git a/internal/service/hdcp_service.go b/internal/service/hdcp_service.go index 8f8bbb0..c7d21d5 100644 --- a/internal/service/hdcp_service.go +++ b/internal/service/hdcp_service.go @@ -1,16 +1,11 @@ package service import ( - "context" - "fmt" - "github.com/thd-spatial-ai/ignis/internal/db/repository" - "github.com/thd-spatial-ai/ignis/internal/hdcp" - "github.com/thd-spatial-ai/ignis/internal/models" "log" "os" - "strings" - "github.com/jackc/pgx/v5/pgxpool" + "github.com/thd-spatial-ai/ignis/internal/hdcp" + "github.com/thd-spatial-ai/ignis/internal/models" ) // Re-export types for easier use in handlers @@ -18,26 +13,16 @@ type TabulaBuildingParameters = models.TabulaBuildingParameters // IgnisService provides business logic for HDCP calculations type IgnisService struct { - logger *hdcp.Logger - repository *repository.BuildingRepository + logger *hdcp.Logger } -// NewIgnisService creates a new HDCP service instance without database +// NewIgnisService creates a new HDCP service instance func NewIgnisService() *IgnisService { return &IgnisService{ logger: hdcp.NewLogger(log.New(os.Stdout, "", 0)), } } -// NewIgnisServiceWithDB creates a new HDCP service with database support. -// schema is the PostgreSQL schema name (e.g. "tabula"). -func NewIgnisServiceWithDB(pool *pgxpool.Pool, schema string) *IgnisService { - return &IgnisService{ - logger: hdcp.NewLogger(log.New(os.Stdout, "", 0)), - repository: repository.NewBuildingRepository(pool, schema), - } -} - // NewIgnisServiceWithLogger creates a new HDCP service with custom logger func NewIgnisServiceWithLogger(logger *hdcp.Logger) *IgnisService { return &IgnisService{ @@ -45,26 +30,6 @@ func NewIgnisServiceWithLogger(logger *hdcp.Logger) *IgnisService { } } -// GetBuildingByCode retrieves building parameters from database by building code -func (s *IgnisService) GetBuildingByCode(ctx context.Context, country, buildingCode string) (*models.TabulaBuildingParameters, error) { - if s.repository == nil { - return nil, fmt.Errorf("database repository not initialized") - } - - // Normalize country name for table name - tableName := normalizeCountryName(country) - - return s.repository.GetByBuildingCode(ctx, tableName, buildingCode) -} - -// normalizeCountryName normalizes country names for table names -func normalizeCountryName(name string) string { - name = strings.ToLower(strings.TrimSpace(name)) - name = strings.ReplaceAll(name, " ", "_") - name = strings.ReplaceAll(name, "-", "_") - return name -} - // CalculateHeatingDemand executes the HDCP calculation pipeline. // Returns the calculated q_h_nd (annual heating energy demand in kWh/(m²·a)). func (s *IgnisService) CalculateHeatingDemand(buildingParams *models.TabulaBuildingParameters) (float64, error) { diff --git a/internal/service/hdcp_service_test.go b/internal/service/hdcp_service_test.go index 6bd28e6..baee455 100644 --- a/internal/service/hdcp_service_test.go +++ b/internal/service/hdcp_service_test.go @@ -1,10 +1,8 @@ package service import ( - "context" "log" "os" - "strings" "testing" "github.com/thd-spatial-ai/ignis/internal/hdcp" @@ -45,17 +43,6 @@ func TestNewIgnisService(t *testing.T) { if s.logger == nil { t.Error("expected non-nil logger") } - if s.repository != nil { - t.Error("expected nil repository when constructed without a DB") - } -} - -func TestNewIgnisServiceWithDB(t *testing.T) { - // NewIgnisServiceWithDB only stores the pool; it never dials, so nil is safe here. - s := NewIgnisServiceWithDB(nil, "tabula") - if s.repository == nil { - t.Error("expected non-nil repository when constructed with a DB") - } } func TestNewIgnisServiceWithLogger(t *testing.T) { @@ -66,30 +53,6 @@ func TestNewIgnisServiceWithLogger(t *testing.T) { } } -func TestGetBuildingByCode_noRepository_returnsError(t *testing.T) { - s := NewIgnisService() - _, err := s.GetBuildingByCode(context.Background(), "germany", "DE.N.SFH.01.Gen") - if err == nil { - t.Fatal("expected error when repository is not initialized") - } - if !strings.Contains(err.Error(), "repository not initialized") { - t.Errorf("error = %q, want mention of uninitialized repository", err.Error()) - } -} - -func TestNormalizeCountryName(t *testing.T) { - cases := []struct{ in, want string }{ - {"Germany", "germany"}, - {" United Kingdom ", "united_kingdom"}, - {"Czech-Republic", "czech_republic"}, - } - for _, tc := range cases { - if got := normalizeCountryName(tc.in); got != tc.want { - t.Errorf("normalizeCountryName(%q) = %q, want %q", tc.in, got, tc.want) - } - } -} - func TestCalculateHeatingDemand_success(t *testing.T) { s := NewIgnisService() // An all-zero building is a division-by-zero case in the pipeline's own