From e4d47147809d6c5d08bc691f21defa370d21d495 Mon Sep 17 00:00:00 2001 From: Bradley Windybank Date: Tue, 7 Jul 2026 13:36:51 +1200 Subject: [PATCH 1/3] Add /collections endpoint, exclude-filtered image picks, and richer metadata Lets the frontend list collections to hide and exclude them from random picks (weights re-normalized over the remaining set, not just filtered client-side), and surfaces creator/date/subject for a fuller "more info" view. --- .../Data Sources/DigitalNZAPIDataSource.swift | 5 +- .../Models/NZRecordsResult.swift | 19 ++++- Sources/NZImageApiLambda/NZImageApi.swift | 30 +++++++- .../NZImageApiLambda/NZImageApiLambda.swift | 24 +++++- .../FilteredCollectionWeightsTests.swift | 76 +++++++++++++++++++ 5 files changed, 149 insertions(+), 5 deletions(-) create mode 100644 Tests/NZImageApiLambdaTests/FilteredCollectionWeightsTests.swift diff --git a/Sources/NZImageApiLambda/Data Sources/DigitalNZAPIDataSource.swift b/Sources/NZImageApiLambda/Data Sources/DigitalNZAPIDataSource.swift index dc3a881..b50de3d 100644 --- a/Sources/NZImageApiLambda/Data Sources/DigitalNZAPIDataSource.swift +++ b/Sources/NZImageApiLambda/Data Sources/DigitalNZAPIDataSource.swift @@ -37,6 +37,7 @@ final class DigitalNZAPIDataSource: Sendable { func newResult( collection: String?, + weightsOverride: OrderedDictionary? = nil, logger: (String) -> Void ) async throws -> NZRecordsResult @@ -47,7 +48,9 @@ final class DigitalNZAPIDataSource: Sendable { chosenCollection = collection } else { - chosenCollection = collectionWeights.weightedRandomPick() + // Use the per-call override (already filtered + renormalized by the caller) when + // provided; otherwise fall back to the unfiltered weights captured at init. + chosenCollection = (weightsOverride ?? collectionWeights).weightedRandomPick() } let secondRequestResultsPerPage = 100 diff --git a/Sources/NZImageApiLambda/Models/NZRecordsResult.swift b/Sources/NZImageApiLambda/Models/NZRecordsResult.swift index 61be42a..3178433 100644 --- a/Sources/NZImageApiLambda/Models/NZRecordsResult.swift +++ b/Sources/NZImageApiLambda/Models/NZRecordsResult.swift @@ -21,7 +21,10 @@ public struct NZRecordsResult: NonNullableResult, Codable, Sendable { collection: String?, landingUrl: URL?, originUrl: URL? = nil, - sourceUrl: URL? = nil + sourceUrl: URL? = nil, + creator: [String]? = nil, + date: [String]? = nil, + subject: [String]? = nil ) { self.id = id self.title = title @@ -33,6 +36,9 @@ public struct NZRecordsResult: NonNullableResult, Codable, Sendable { self.landingUrl = landingUrl self.originUrl = originUrl self.sourceUrl = sourceUrl + self.creator = creator + self.date = date + self.subject = subject } // MARK: Internal @@ -62,6 +68,9 @@ public struct NZRecordsResult: NonNullableResult, Codable, Sendable { case landingUrl = "landing_url" case originUrl = "origin_url" case sourceUrl = "source_url" + case creator + case date + case subject } public var id: Int? @@ -74,6 +83,9 @@ public struct NZRecordsResult: NonNullableResult, Codable, Sendable { public var landingUrl: URL? public var originUrl: URL? public var sourceUrl: URL? + public var creator: [String]? + public var date: [String]? + public var subject: [String]? func customDescription() -> String { return """ @@ -86,7 +98,10 @@ public struct NZRecordsResult: NonNullableResult, Codable, Sendable { collection: \(String(describing: collection)), landingUrl: \(String(describing: landingUrl)), originUrl: \(String(describing: originUrl)), - sourceUrl: \(String(describing: sourceUrl)) + sourceUrl: \(String(describing: sourceUrl)), + creator: \(String(describing: creator)), + date: \(String(describing: date)), + subject: \(String(describing: subject)) """ } diff --git a/Sources/NZImageApiLambda/NZImageApi.swift b/Sources/NZImageApiLambda/NZImageApi.swift index 492ecc1..1863997 100644 --- a/Sources/NZImageApiLambda/NZImageApi.swift +++ b/Sources/NZImageApiLambda/NZImageApi.swift @@ -95,10 +95,38 @@ public struct NZImageApi: Sendable { return collectionImageCounts.mapValues { Double($0) / total } } - public func image(collection: String?, logger: @Sendable (String) -> Void = { _ in }) async -> NZRecordsResult? { + /// Thrown when `exclude` removes every collection, leaving nothing eligible to pick from. + /// Surfaced to the handler so it can return a `.badRequest` rather than silently falling + /// back to the full unfiltered set. + public struct NoEligibleCollectionsError: Error {} + + /// Weights over `collectionImageCounts` minus `exclude`, re-derived as `count / filtered-total` + /// so they still sum to 1 (mod float rounding) over the remaining set — never + /// subtract-then-leave-unnormalized, which reintroduces the documented "weights don't sum to 1" + /// bug. Throws `NoEligibleCollectionsError` if `exclude` removes every collection. + public static func filteredCollectionWeights(excluding exclude: Set) throws -> OrderedDictionary { + let filteredCounts = collectionImageCounts.filter { !exclude.contains($0.key) } + guard !filteredCounts.isEmpty else { throw NoEligibleCollectionsError() } + let total = Double(filteredCounts.values.reduce(0, +)) + return filteredCounts.mapValues { Double($0) / total } + } + + public func image( + collection: String?, + exclude: Set = [], + logger: @Sendable (String) -> Void = { _ in } + ) async -> NZRecordsResult? { do { + // An explicit single collection always wins; `exclude` is only consulted for the + // random path. When `exclude` is non-empty we filter+renormalize the canonical counts. + var weightsOverride: OrderedDictionary? = nil + if collection == nil, !exclude.isEmpty { + weightsOverride = try NZImageApi.filteredCollectionWeights(excluding: exclude) + } + let result = try await digitalNZAPIDataSource.newResult( collection: collection, + weightsOverride: weightsOverride, logger: logger ) return result diff --git a/Sources/NZImageApiLambda/NZImageApiLambda.swift b/Sources/NZImageApiLambda/NZImageApiLambda.swift index c44c3c7..229527c 100644 --- a/Sources/NZImageApiLambda/NZImageApiLambda.swift +++ b/Sources/NZImageApiLambda/NZImageApiLambda.swift @@ -28,12 +28,34 @@ struct NZImageApiLambda { context.logger.log(level: .info, "Authorized consumer: \(consumer)") switch (event.context.http.path, event.context.http.method) { + case ("/collections", .get): + let jsonEncoder = JSONEncoder() + jsonEncoder.outputFormatting = .withoutEscapingSlashes + let jsonData = try jsonEncoder.encode(Array(NZImageApi.collectionImageCounts.keys)) + let jsonString = String(data: jsonData, encoding: .utf8) + + return APIGatewayV2Response(statusCode: .ok, headers: ["content-type": "application/json"], body: jsonString) + case ("/image", .get): let requestedCollection = event.queryStringParameters["collection"] + // `collection` (single explicit override) wins over `exclude`; only consult `exclude` + // when no explicit collection was requested. + let exclude: Set + if requestedCollection != nil { + exclude = [] + } + else if let excludeParam = event.queryStringParameters["exclude"], !excludeParam.isEmpty { + exclude = Set(excludeParam.split(separator: ",").map(String.init)) + } + else { + exclude = [] + } + context.logger.log(level: .info, "Requested collection: \(requestedCollection ?? "random")") + context.logger.log(level: .info, "Excluded collections: \(exclude.isEmpty ? "none" : exclude.sorted().joined(separator: ", "))") - guard let image = await api.image(collection: requestedCollection, logger: { log in + guard let image = await api.image(collection: requestedCollection, exclude: exclude, logger: { log in context.logger.log(level: .info, "\(log)") }) else { context.logger.log(level: .error, "Failed to get image for collection: \(requestedCollection ?? "random")") diff --git a/Tests/NZImageApiLambdaTests/FilteredCollectionWeightsTests.swift b/Tests/NZImageApiLambdaTests/FilteredCollectionWeightsTests.swift new file mode 100644 index 0000000..9e55848 --- /dev/null +++ b/Tests/NZImageApiLambdaTests/FilteredCollectionWeightsTests.swift @@ -0,0 +1,76 @@ +// +// FilteredCollectionWeightsTests.swift +// +// Coverage for the `?exclude=` renormalization path (`NZImageApi.filteredCollectionWeights`). +// The invariant mirrors CollectionWeightsTests but over the FILTERED set: dropping collections +// must re-derive weights as `count / filtered-total` so they still sum to 1, rather than +// subtracting-then-leaving-unnormalized (which would reintroduce the weightedRandomPick +// fallthrough bug the unfiltered path already guards against). +// + +import OrderedCollections +import XCTest +@testable import NZImageApiLambda + +final class FilteredCollectionWeightsTests: XCTestCase { + func testFilteredWeightsSumToOne() throws { + let exclude: Set = ["Te Papa Collections Online", "Kura Heritage Collections Online"] + let weights = try NZImageApi.filteredCollectionWeights(excluding: exclude) + let total = weights.values.reduce(0, +) + XCTAssertEqual(total, 1.0, accuracy: 0.000_001) + } + + func testExcludedCollectionsAreAbsent() throws { + let exclude: Set = ["Te Papa Collections Online", "Kura Heritage Collections Online"] + let weights = try NZImageApi.filteredCollectionWeights(excluding: exclude) + for excluded in exclude { + XCTAssertNil(weights[excluded], "\(excluded) should not appear in the filtered weights") + } + } + + func testFilteredWeightsPreserveRemainingCollections() throws { + let exclude: Set = ["Te Papa Collections Online"] + let weights = try NZImageApi.filteredCollectionWeights(excluding: exclude) + XCTAssertEqual( + weights.count, + NZImageApi.collectionImageCounts.count - exclude.count + ) + // A large non-excluded collection must still be present with a positive weight, proving + // the remaining proportions were renormalized rather than dropped. + XCTAssertNotNil(weights["Kura Heritage Collections Online"]) + for (name, weight) in weights { + XCTAssertGreaterThan(weight, 0, "\(name) has a non-positive weight") + } + } + + func testRenormalizationRedistributesProportionally() throws { + // Removing a collection should scale every survivor's weight up by exactly + // 1 / (1 - removedShare), preserving relative proportions among survivors. + let removed = "Te Papa Collections Online" + let unfiltered = NZImageApi.collectionWeights + let removedShare = unfiltered[removed]! + let filtered = try NZImageApi.filteredCollectionWeights(excluding: [removed]) + + let scale = 1.0 / (1.0 - removedShare) + for (name, filteredWeight) in filtered { + XCTAssertEqual(filteredWeight, unfiltered[name]! * scale, accuracy: 0.000_001) + } + } + + func testExcludingEveryCollectionThrows() { + let all = Set(NZImageApi.collectionImageCounts.keys) + XCTAssertThrowsError(try NZImageApi.filteredCollectionWeights(excluding: all)) { error in + XCTAssertTrue(error is NZImageApi.NoEligibleCollectionsError) + } + } + + func testEmptyExcludeMatchesUnfilteredWeights() throws { + // An empty exclude set is a degenerate filter: it must reproduce the canonical + // unfiltered weights exactly (same keys, same order, same values). + let filtered = try NZImageApi.filteredCollectionWeights(excluding: []) + XCTAssertEqual(Array(filtered.keys), Array(NZImageApi.collectionWeights.keys)) + for (name, weight) in filtered { + XCTAssertEqual(weight, NZImageApi.collectionWeights[name]!, accuracy: 0.000_001) + } + } +} From 7a9a4eb2de0c13c2fc2b432073865ecd2363dfe3 Mon Sep 17 00:00:00 2001 From: Bradley Windybank Date: Tue, 7 Jul 2026 13:45:29 +1200 Subject: [PATCH 2/3] Register the /collections route on the HTTP API The Swift handler already switched on it, but SAM's HttpApi only wires up explicitly declared routes -- without this the API Gateway itself 404s before the request ever reaches the Lambda. --- template.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/template.yaml b/template.yaml index 1261c35..8a4eca3 100644 --- a/template.yaml +++ b/template.yaml @@ -173,6 +173,11 @@ Resources: Properties: Path: /image Method: GET + Collections: + Type: HttpApi + Properties: + Path: /collections + Method: GET Metadata: BuildMethod: makefile From 6c5a1f2e3606ff92f0ae7e4165bd744a382314f0 Mon Sep 17 00:00:00 2001 From: Bradley Windybank Date: Fri, 10 Jul 2026 00:10:22 +1200 Subject: [PATCH 3/3] Log NoEligibleCollectionsError distinctly from unexpected errors CodeRabbit review on #32: falling through to the generic catch made an expected, user-triggerable condition (an exclude list covering every collection) read as a genuine bug in the logs. --- Sources/NZImageApiLambda/NZImageApi.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Sources/NZImageApiLambda/NZImageApi.swift b/Sources/NZImageApiLambda/NZImageApi.swift index 1863997..e997fef 100644 --- a/Sources/NZImageApiLambda/NZImageApi.swift +++ b/Sources/NZImageApiLambda/NZImageApi.swift @@ -131,6 +131,11 @@ public struct NZImageApi: Sendable { ) return result } + catch is NoEligibleCollectionsError { + // Expected, user-triggerable (an `exclude` list that covers every collection) -- + // logged distinctly from the catch-all below so it doesn't read as a genuine bug. + logger("No eligible collections remain after applying the exclude filter") + } catch { if let richError = error as? (any RichError) { // Get the raw value from the enum that defines the kind of error. Messy due to RichError being a protocol and the nested