From 7c5a03b0065debc89c517ef71b1b91958ab92682 Mon Sep 17 00:00:00 2001 From: Jay Ravani Date: Tue, 8 Sep 2026 05:37:19 +0200 Subject: [PATCH] feat(api): clamp out-of-range construction year to nearest TABULA period GET /api/v1/variants/{iso2}/match?type=&year= returned an empty match list whenever no period band for the requested type covered the given year. 27 of 93 country+type period sets in the TABULA workbook have no open-ended (0 or 9999) sentinel band at one or both ends, so a year before or after the type's recorded range never matched anything (DE.N.AB only covers 1860-1978, BE.N.AB only covers 1946 onward). ResolvePeriodByYear now clamps a year outside every band for a type to the nearest edge: below the oldest band resolves to the oldest period, above the newest resolves to the newest. A year inside the type's overall range but falling in a gap between two defined bands is left as a genuine no-match, since which neighbouring band to prefer there is undefined. Also add input guards on the year query parameter: it must be a non-negative integer and must not exceed the current calendar year. Closes #34 --- docs/api.md | 2 +- docs/openapi/openapi.yaml | 10 ++- internal/api/handler/data.go | 17 ++++- internal/api/handler/handler_helpers_test.go | 31 +++++++++ .../repository/repository_integration_test.go | 17 +++-- internal/db/repository/tabula_repository.go | 68 +++++++++++++++++-- 6 files changed, 128 insertions(+), 17 deletions(-) diff --git a/docs/api.md b/docs/api.md index f196bd3..6a5fd41 100644 --- a/docs/api.md +++ b/docs/api.md @@ -22,7 +22,7 @@ ignis has no authentication of its own. The reverse proxy in front of it is the Step 2 is optional. Call it when you need to inspect or report the inputs; `calculate` uses them either way. -If you have a construction year rather than a period code, send `year=` in place of `period=` at step 1 and ignis resolves it. Exactly one of the two is required. `GET /api/v1/periods/{country}` lists a country's bands as `{ period, year_from, year_to }`, oldest first, with `year_from` 0 and `year_to` 9999 marking the open-ended bands. +If you have a construction year rather than a period code, send `year=` in place of `period=` at step 1 and ignis resolves it. Exactly one of the two is required. `year` must be a non-negative integer no later than the current calendar year. A year before the earliest or after the latest band defined for that country and type resolves to the oldest or newest period. `GET /api/v1/periods/{country}` lists a country's bands as `{ period, year_from, year_to }`, oldest first, with `year_from` 0 and `year_to` 9999 marking the open-ended bands. ## Overriding archetype inputs diff --git a/docs/openapi/openapi.yaml b/docs/openapi/openapi.yaml index 6096b83..0c05be0 100644 --- a/docs/openapi/openapi.yaml +++ b/docs/openapi/openapi.yaml @@ -105,8 +105,10 @@ paths: period, ordered from existing state to most refurbished. Give the period either directly (period) or as a construction year (year), which ignis resolves to the period whose band contains it for that country and type. - A year with no matching archetype for that type returns an empty data - list. + A year before the earliest or after the latest band defined for that + type resolves to the oldest or newest period rather than failing to + match. A year that falls in a gap between two defined bands, or a type + with no bands at all, returns an empty data list. parameters: - $ref: "#/components/parameters/CountryIso2" - name: type @@ -131,9 +133,11 @@ paths: required: false description: > Construction year, resolved to a period. Exactly one of period or - year is required. + year is required. Must be a non-negative integer no later than the + current calendar year. schema: type: integer + minimum: 0 example: 1975 responses: "200": diff --git a/internal/api/handler/data.go b/internal/api/handler/data.go index 2ebad59..4b9a7f9 100644 --- a/internal/api/handler/data.go +++ b/internal/api/handler/data.go @@ -62,9 +62,12 @@ func refurbishmentLabel(index int) string { // MatchVariants returns all refurbishment variants for a building type and construction period. // Query params: type (e.g. SFH) is required; exactly one of period (e.g. 01) or -// year (e.g. 1975) must be given. With year, ignis resolves the period whose band -// contains it for that country and type. The response is ordered from existing -// state to most-refurbished. +// year (e.g. 1975) must be given. year must be a non-negative integer no later +// than the current calendar year. With year, ignis resolves the period whose +// band contains it for that country and type; a year outside every defined +// band for that type clamps to the oldest or newest period rather than +// failing to match. The response is ordered from existing state to +// most-refurbished. func (h *Handler) MatchVariants(c *gin.Context) { isoCode := strings.ToUpper(strings.TrimSpace(c.Param("country_iso2"))) tableName, err := tableNameFromISO(isoCode) @@ -98,6 +101,14 @@ func (h *Handler) MatchVariants(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("query param 'year' must be an integer, got %q", yearParam)}) return } + if year < 0 { + c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("query param 'year' must not be negative, got %d", year)}) + return + } + if currentYear := time.Now().Year(); year > currentYear { + c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("query param 'year' must not be in the future, got %d", year)}) + return + } period, err = h.repo.ResolvePeriodByYear(ctx, tableName, typePrefix, year) if err != nil { utils.Error.Printf("failed to resolve period for %s year %d: %v", typePrefix, year, err) diff --git a/internal/api/handler/handler_helpers_test.go b/internal/api/handler/handler_helpers_test.go index cb82709..b8ecbbb 100644 --- a/internal/api/handler/handler_helpers_test.go +++ b/internal/api/handler/handler_helpers_test.go @@ -4,9 +4,11 @@ import ( "context" "encoding/json" "errors" + "fmt" "net/http" "strings" "testing" + "time" "github.com/thd-spatial-ai/ignis/internal/db/repository" ) @@ -275,6 +277,35 @@ func TestMatchVariants_nonIntegerYear_returns400(t *testing.T) { } } +func TestMatchVariants_negativeYear_returns400(t *testing.T) { + mock := &mockRepo{ + resolvePeriodByYear: func(_ context.Context, _, _ string, _ int) (string, error) { + t.Error("ResolvePeriodByYear must not be called for a negative year") + return "", nil + }, + } + h := newTestHandler(mock) + w := serve(http.MethodGet, "/variants/DE/match?type=SFH&year=-5", "/variants/:country_iso2/match", h.MatchVariants, nil) + if w.Code != http.StatusBadRequest { + t.Errorf("expected 400 for negative year, got %d", w.Code) + } +} + +func TestMatchVariants_futureYear_returns400(t *testing.T) { + mock := &mockRepo{ + resolvePeriodByYear: func(_ context.Context, _, _ string, _ int) (string, error) { + t.Error("ResolvePeriodByYear must not be called for a future year") + return "", nil + }, + } + h := newTestHandler(mock) + futureYear := time.Now().Year() + 1 + w := serve(http.MethodGet, fmt.Sprintf("/variants/DE/match?type=SFH&year=%d", futureYear), "/variants/:country_iso2/match", h.MatchVariants, nil) + if w.Code != http.StatusBadRequest { + t.Errorf("expected 400 for a year in the future, got %d", w.Code) + } +} + func TestMatchVariants_yearWithNoMatchingArchetype_returns200EmptyList(t *testing.T) { mock := &mockRepo{ resolvePeriodByYear: func(_ context.Context, _, _ string, _ int) (string, error) { diff --git a/internal/db/repository/repository_integration_test.go b/internal/db/repository/repository_integration_test.go index 324a495..82d1ff0 100644 --- a/internal/db/repository/repository_integration_test.go +++ b/internal/db/repository/repository_integration_test.go @@ -107,7 +107,9 @@ func seedFixtures(ctx context.Context, pool *pgxpool.Pool) error { ('DE.N.SFH.01.ReEx', 75.5, 185, 98.70), ('DE.N.MFH.01.Gen', 200.0, 185, 88.10)`, // austria carries the construction-year bands for the period-resolution tests. - // MFH deliberately has no 01 band, so year resolution is scoped to type. + // MFH deliberately has no 01 band and no open-ended sentinel at either + // end, so it can stand in for a type whose bands don't cover 0..9999. + // TH deliberately has a gap (1919-1999) between its two bands. `CREATE TABLE tabula.austria ( id SERIAL PRIMARY KEY, "Code_BuildingVariant" VARCHAR, @@ -120,7 +122,9 @@ func seedFixtures(ctx context.Context, pool *pgxpool.Pool) error { ('AT.N.SFH.01.ReEx', 'AT.01', 0, 1918), ('AT.N.SFH.02.Gen', 'AT.02', 1919, 1944), ('AT.N.SFH.12.Gen', 'AT.12', 2009, 9999), - ('AT.N.MFH.02.Gen', 'AT.02', 1919, 1944)`, + ('AT.N.MFH.02.Gen', 'AT.02', 1919, 1944), + ('AT.N.TH.01.Gen', 'AT.01', 0, 1918), + ('AT.N.TH.05.Gen', 'AT.05', 2000, 9999)`, } for _, stmt := range statements { if _, err := pool.Exec(ctx, stmt); err != nil { @@ -191,8 +195,12 @@ func TestTabulaRepository_ResolvePeriodByYear(t *testing.T) { {"upper boundary of oldest band", "AT.N.SFH", 1918, "01"}, {"lower boundary of next band", "AT.N.SFH", 1919, "02"}, {"open-ended newest band", "AT.N.SFH", 2050, "12"}, - {"type without that band", "AT.N.MFH", 1900, ""}, - {"year in no band for type", "AT.N.MFH", 3000, ""}, + {"year before the type's only band clamps to it", "AT.N.MFH", 1900, "02"}, + {"year after the type's only band clamps to it", "AT.N.MFH", 3000, "02"}, + {"year before every band for the type clamps to oldest", "AT.N.TH", 1800, "01"}, + {"year after every band for the type clamps to newest", "AT.N.TH", 2050, "05"}, + {"year in a gap between two defined bands is not clamped", "AT.N.TH", 1950, ""}, + {"type with no bands at all", "AT.N.AB", 1950, ""}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -216,6 +224,7 @@ func TestTabulaRepository_ListPeriods(t *testing.T) { want := []repository.ConstructionPeriod{ {Period: "01", YearFrom: 0, YearTo: 1918}, {Period: "02", YearFrom: 1919, YearTo: 1944}, + {Period: "05", YearFrom: 2000, YearTo: 9999}, {Period: "12", YearFrom: 2009, YearTo: 9999}, } if len(periods) != len(want) { diff --git a/internal/db/repository/tabula_repository.go b/internal/db/repository/tabula_repository.go index 61de22d..e123e3d 100644 --- a/internal/db/repository/tabula_repository.go +++ b/internal/db/repository/tabula_repository.go @@ -103,9 +103,12 @@ func periodFromYearClass(yearClass string) string { // ResolvePeriodByYear returns the period index whose construction-year band // contains year, scoped to one building type. typePrefix is "CC.N.TYPE". -// Year1_Building 0 and Year2_Building 9999 are the open-ended sentinels; the -// plain range comparison covers both without special-casing. Returns "" when no -// band for that type contains the year. +// Year1_Building 0 and Year2_Building 9999 are the open-ended sentinels some +// countries use; others simply stop at their oldest or newest recorded +// period, so a year before/after every band for that type clamps to the +// nearest edge (see clampToNearestPeriod) rather than returning no match. +// Returns "" only when the type has no bands at all, or the year falls in a +// gap between two defined bands. func (r *TabulaRepository) ResolvePeriodByYear(ctx context.Context, tableName, typePrefix string, year int) (string, error) { query := fmt.Sprintf( `SELECT "Code_ConstructionYearClass" FROM %s @@ -117,16 +120,69 @@ func (r *TabulaRepository) ResolvePeriodByYear(ctx context.Context, tableName, t var yearClass string err := r.pool.QueryRow(ctx, query, typePrefix+".%", year).Scan(&yearClass) - if errors.Is(err, pgx.ErrNoRows) { - return "", nil + if err == nil { + return periodFromYearClass(yearClass), nil } - if err != nil { + if !errors.Is(err, pgx.ErrNoRows) { return "", fmt.Errorf("failed to resolve period for %s year %d: %w", typePrefix, year, err) } + yearClass, err = r.clampToNearestPeriod(ctx, tableName, typePrefix, year) + if err != nil { + return "", err + } + if yearClass == "" { + return "", nil + } return periodFromYearClass(yearClass), nil } +// clampToNearestPeriod handles a year that matched no band for typePrefix. If +// year precedes every defined band it returns the oldest one; if it follows +// every band it returns the newest. A year that falls inside the type's +// overall range but between two defined bands (a gap) is left as "" — which +// neighbour to prefer there is undefined, so it stays a genuine no-match. +func (r *TabulaRepository) clampToNearestPeriod(ctx context.Context, tableName, typePrefix string, year int) (string, error) { + boundsQuery := fmt.Sprintf( + `SELECT MIN("Year1_Building"), MAX("Year2_Building") FROM %s WHERE "Code_BuildingVariant" LIKE $1`, + r.qualifyTable(tableName), + ) + + var minYear1, maxYear2 *int + if err := r.pool.QueryRow(ctx, boundsQuery, typePrefix+".%").Scan(&minYear1, &maxYear2); err != nil { + return "", fmt.Errorf("failed to load year bounds for %s: %w", typePrefix, err) + } + if minYear1 == nil || maxYear2 == nil { + return "", nil // no bands at all for this type + } + + var edgeQuery string + switch { + case year < *minYear1: + edgeQuery = fmt.Sprintf( + `SELECT "Code_ConstructionYearClass" FROM %s + WHERE "Code_BuildingVariant" LIKE $1 + ORDER BY "Year1_Building" ASC LIMIT 1`, + r.qualifyTable(tableName), + ) + case year > *maxYear2: + edgeQuery = fmt.Sprintf( + `SELECT "Code_ConstructionYearClass" FROM %s + WHERE "Code_BuildingVariant" LIKE $1 + ORDER BY "Year2_Building" DESC LIMIT 1`, + r.qualifyTable(tableName), + ) + default: + return "", nil // inside the overall range but in a gap between bands + } + + var yearClass string + if err := r.pool.QueryRow(ctx, edgeQuery, typePrefix+".%").Scan(&yearClass); err != nil { + return "", fmt.Errorf("failed to clamp period for %s year %d: %w", typePrefix, year, err) + } + return yearClass, nil +} + // ListPeriods returns the country's distinct construction-year bands, oldest first. func (r *TabulaRepository) ListPeriods(ctx context.Context, tableName string) ([]ConstructionPeriod, error) { query := fmt.Sprintf(