fix: apply the from and to parameters where the spec declares them

CHANGELOG.md

@@ -45,6 +45,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   delete event and notified the webhooks.
 - The event recorded by an analysis `PATCH` had no actor.
 - `?from` and `?to` refused RFC 3339 values with a zone offset.
+- `?from` and `?to` were ignored on the per entity log lists and on the
+  webhook lists, although the OpenAPI spec declares them there.
 - Catalogs, bundles and webhooks wrote no event to the audit trail.
 - A webhook `POST` with the url of an existing one answered `500` instead
   of `409`.

bundles_test.go

@@ -38,6 +38,17 @@ func TestBundleEndpoints(t *testing.T) {
 				assertOnlyKeys(t, first, "id", "name", "description", "active", "softwareIds", "createdAt", "updatedAt")
 			},
 		},
+		{
+			description:         "GET bundles ignores a from param the operation does not declare",
+			query:               "GET /v1/bundles?from=not-a-date",
+			expectedCode:        200,
+			expectedContentType: "application/json",
+			validateFunc: func(t *testing.T, response map[string]interface{}) {
+				data := assertListResponse(t, response)
+
+				assert.Equal(t, 2, len(data))
+			},
+		},
 		{
 			description:         "GET bundles with all=true returns inactive too",
 			query:               "GET /v1/bundles?all=true",

catalogs_test.go

@@ -74,6 +74,18 @@ func TestCatalogEndpoints(t *testing.T) {
 			},
 		},
 
+		{
+			description:         "GET catalogs ignores a from param the operation does not declare",
+			query:               "GET /v1/catalogs?from=not-a-date",
+			expectedCode:        200,
+			expectedContentType: "application/json",
+			validateFunc: func(t *testing.T, response map[string]any) {
+				data := assertListResponse(t, response)
+
+				assert.Equal(t, 2, len(data))
+			},
+		},
+
 		// GET /catalogs/:id
 		{
 			description:         "GET catalog by id",
@@ -705,13 +717,14 @@ func TestCatalogEndpoints(t *testing.T) {
 			},
 		},
 		{
-			description:         "GET catalog software with unknown url and an invalid from param still validates from",
-			query:               "GET /v1/catalogs/" + italiaID + "/software?url=https://example.org/unknown&from=not-a-date",
-			expectedCode:        422,
-			expectedContentType: "application/problem+json",
+			description:         "GET catalog software ignores a from param the operation does not declare",
+			query:               "GET /v1/catalogs/" + italiaID + "/software?from=not-a-date",
+			expectedCode:        200,
+			expectedContentType: "application/json",
 			validateFunc: func(t *testing.T, response map[string]any) {
-				assert.Equal(t, `can't get Software`, response["title"])
-				assert.Equal(t, "invalid date time format (RFC 3339 needed)", response["detail"])
+				data := assertListResponse(t, response)
+
+				assert.Equal(t, 1, len(data))
 			},
 		},
 		{

internal/handlers/catalogs.go

@@ -378,10 +378,7 @@ func (c *Catalog) GetCatalogSoftware(ctx *fiber.Ctx) error {
 		return err
 	}
 
-	stmt, err := general.Clauses(ctx, c.db.Preload("Aliases").Scopes(catalogScope(catalog)), "", "")
-	if err != nil {
-		return common.Error(fiber.StatusUnprocessableEntity, "can't get Software", general.QueryErrorDetail(err))
-	}
+	stmt := c.db.Preload("Aliases").Scopes(catalogScope(catalog))
 
 	stmt, found, err := softwareURLFilter(ctx, c.db, stmt, "can't get Software")
 	if err != nil {

internal/handlers/crud.go

@@ -28,11 +28,11 @@ type listOptions struct {
 	order paginator.Order
 	// activeOnly applies models.Active unless ?all=true is given.
 	activeOnly bool
-	// skipClauses leaves ?filter, ?from, ?to and ?search untouched,
-	// either because the list never supported them (webhooks, per
-	// resource logs) or because the handler applied them itself before
-	// a short circuit (the software lists).
-	skipClauses bool
+	// dateWindow narrows the list to the ?from and ?to window on
+	// created_at. Only the lists whose OpenAPI operation declares the two
+	// parameters read them, elsewhere they are ignored rather than
+	// filtering a page or answering 422 on a value nobody documented.
+	dateWindow bool
 }
 
 // paginate applies opts to stmt and returns one page of T plus the cursor
@@ -41,17 +41,13 @@ type listOptions struct {
 func paginate[T any](ctx *fiber.Ctx, stmt *gorm.DB, opts listOptions) ([]T, paginator.Cursor, error) {
 	var items []T
 
-	if !opts.skipClauses {
-		var err error
-
-		stmt, err = general.Clauses(ctx, stmt, opts.filterField, opts.searchField)
-		if err != nil {
-			return nil, paginator.Cursor{}, common.Error(
-				fiber.StatusUnprocessableEntity,
-				opts.title,
-				general.QueryErrorDetail(err),
-			)
-		}
+	stmt, err := general.Clauses(ctx, stmt, opts.filterField, opts.searchField, opts.dateWindow)
+	if err != nil {
+		return nil, paginator.Cursor{}, common.Error(
+			fiber.StatusUnprocessableEntity,
+			opts.title,
+			general.QueryErrorDetail(err),
+		)
 	}
 
 	if opts.activeOnly && !ctx.QueryBool("all", false) {

internal/handlers/crud_test.go

@@ -110,7 +110,7 @@ func TestPaginateFilterField(t *testing.T) {
 	require.Len(t, items, 1)
 	assert.Equal(t, "two", items[0].ID)
 
-	items, _, err = paginate[crudItem](newCrudCtx(t, "filter=second"), db, listOptions{title: "t", skipClauses: true})
+	items, _, err = paginate[crudItem](newCrudCtx(t, "filter=second"), db, listOptions{title: "t"})
 	require.NoError(t, err)
 	assert.Len(t, items, 3)
 }
@@ -131,11 +131,34 @@ func TestPaginateSearchField(t *testing.T) {
 	assert.Len(t, items, 3)
 }
 
+func TestPaginateDateWindow(t *testing.T) {
+	db := newCrudDB(t)
+	seedCrudItems(t, db)
+
+	items, _, err := paginate[crudItem](
+		newCrudCtx(t, "from=2024-01-01T00:30:00Z"), db, listOptions{title: "t", dateWindow: true},
+	)
+	require.NoError(t, err)
+	assert.Len(t, items, 2)
+
+	// Without the window the two parameters are ignored, a bad value
+	// included: the list does not document them.
+	items, _, err = paginate[crudItem](newCrudCtx(t, "from=2024-01-01T00:30:00Z"), db, listOptions{title: "t"})
+	require.NoError(t, err)
+	assert.Len(t, items, 3)
+
+	items, _, err = paginate[crudItem](newCrudCtx(t, "from=not-a-date"), db, listOptions{title: "t"})
+	require.NoError(t, err)
+	assert.Len(t, items, 3)
+}
+
 func TestPaginateErrors(t *testing.T) {
 	db := newCrudDB(t)
 	seedCrudItems(t, db)
 
-	_, _, err := paginate[crudItem](newCrudCtx(t, "from=not-a-date"), db, listOptions{title: "can't get Items"})
+	_, _, err := paginate[crudItem](
+		newCrudCtx(t, "from=not-a-date"), db, listOptions{title: "can't get Items", dateWindow: true},
+	)
 	assert.Equal(t, fiber.StatusUnprocessableEntity, problemStatus(t, err))
 
 	_, _, err = paginate[crudItem](newCrudCtx(t, "page[after]=garbage"), db, listOptions{title: "can't get Items"})

internal/handlers/events.go

@@ -46,7 +46,7 @@ func (e *Event) GetEvents(ctx *fiber.Ctx) error {
 		stmt = stmt.Where(map[string]any{"type": eventType})
 	}
 
-	return list[models.Event](ctx, stmt, listOptions{title: errMsg, order: paginator.DESC})
+	return list[models.Event](ctx, stmt, listOptions{title: errMsg, order: paginator.DESC, dateWindow: true})
 }
 
 // GetEvent gets the event with the given ID and returns any error encountered.

internal/handlers/general/query.go

@@ -36,9 +36,10 @@ func QueryErrorDetail(err error) string {
 // Clauses applies the ?filter, ?from, ?to and ?search query parameters
 // to stmt. filterField is the column ?filter matches exactly and
 // searchField the column ?search matches as a case insensitive
-// substring. An empty column name leaves its parameter unread: the
-// lists without such a column must not pass it on to the database.
-func Clauses(ctx *fiber.Ctx, stmt *gorm.DB, filterField, searchField string) (*gorm.DB, error) {
+// substring, dateWindow turns ?from and ?to on. An empty column name or
+// a false dateWindow leaves the parameter unread: a list must not act
+// on a parameter its OpenAPI operation does not declare.
+func Clauses(ctx *fiber.Ctx, stmt *gorm.DB, filterField, searchField string, dateWindow bool) (*gorm.DB, error) {
 	ret := stmt
 
 	if filterField != "" {
@@ -49,6 +50,31 @@ func Clauses(ctx *fiber.Ctx, stmt *gorm.DB, filterField, searchField string) (*g
 		}
 	}
 
+	if dateWindow {
+		var err error
+
+		ret, err = createdAtWindow(ctx, ret)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	if searchField != "" {
+		if search := ctx.Query("search", ""); search != "" {
+			ret = ret.Where(clause.Expr{
+				SQL:  "LOWER(?) LIKE ?",
+				Vars: []any{clause.Column{Name: searchField}, "%" + utils.ToLower(search) + "%"},
+			})
+		}
+	}
+
+	return ret, nil
+}
+
+// createdAtWindow narrows stmt to the ?from and ?to window.
+func createdAtWindow(ctx *fiber.Ctx, stmt *gorm.DB) (*gorm.DB, error) {
+	ret := stmt
+
 	if from := ctx.Query("from", ""); from != "" {
 		at, err := time.Parse(time.RFC3339, from)
 		if err != nil {
@@ -67,14 +93,5 @@ func Clauses(ctx *fiber.Ctx, stmt *gorm.DB, filterField, searchField string) (*g
 		ret = ret.Where("created_at < ?", at)
 	}
 
-	if searchField != "" {
-		if search := ctx.Query("search", ""); search != "" {
-			ret = ret.Where(clause.Expr{
-				SQL:  "LOWER(?) LIKE ?",
-				Vars: []any{clause.Column{Name: searchField}, "%" + utils.ToLower(search) + "%"},
-			})
-		}
-	}
-
 	return ret, nil
 }

internal/handlers/general/query_search_test.go

@@ -20,7 +20,7 @@ func TestClausesQuotesSearchColumn(t *testing.T) {
 
 	stmt := db.Session(&gorm.Session{DryRun: true}).Table("logs")
 
-	ret, err := Clauses(ctx, stmt, "", "name")
+	ret, err := Clauses(ctx, stmt, "", "name", false)
 	require.NoError(t, err)
 
 	sql := ret.Find(&[]struct{}{}).Statement.SQL.String()

internal/handlers/logs.go

@@ -26,6 +26,7 @@ func (p *Log) GetLogs(ctx *fiber.Ctx) error {
 		filterField: "message",
 		searchField: "message",
 		order:       paginator.DESC,
+		dateWindow:  true,
 	})
 }
 
@@ -179,9 +180,9 @@ func (p *Log) getEntityLogs(ctx *fiber.Ctx, entity models.Model, entityName stri
 		Where("entity_id = ?", entity.UUID())
 
 	return list[models.Log](ctx, stmt, listOptions{
-		title:       "can't get Logs",
-		order:       paginator.DESC,
-		skipClauses: true,
+		title:      "can't get Logs",
+		order:      paginator.DESC,
+		dateWindow: true,
 	})
 }
 

internal/handlers/publishers.go

@@ -27,6 +27,7 @@ func (p *Publisher) GetPublishers(ctx *fiber.Ctx) error {
 	return list[models.Publisher](ctx, p.db.Preload(codeHostingAssociation), listOptions{
 		title:      "can't get Publishers",
 		activeOnly: true,
+		dateWindow: true,
 	})
 }
 

internal/handlers/software.go

@@ -30,7 +30,7 @@ func NewSoftware(db *gorm.DB) *Software {
 
 // GetAllSoftware gets the list of all software and returns any error encountered.
 func (p *Software) GetAllSoftware(ctx *fiber.Ctx) error {
-	stmt, err := general.Clauses(ctx, p.db.Preload("Aliases"), "", "")
+	stmt, err := general.Clauses(ctx, p.db.Preload("Aliases"), "", "", true)
 	if err != nil {
 		return common.Error(fiber.StatusUnprocessableEntity, "can't get Software", general.QueryErrorDetail(err))
 	}
@@ -375,12 +375,13 @@ func softwareURLFilter(ctx *fiber.Ctx, gormdb, stmt *gorm.DB, title string) (*go
 
 // listSoftware writes one page of the software matched by stmt. stmt must
 // Preload("Aliases") and nothing else: Preload("URL") makes gorm guess a
-// has one through the software_id column and load an arbitrary url.
+// has one through the software_id column and load an arbitrary url. The
+// ?from and ?to window is the caller's, GET /software applies it before
+// its ?url short circuit and the catalog software list does not declare it.
 func listSoftware(ctx *fiber.Ctx, stmt *gorm.DB) error {
 	software, cursor, err := paginate[models.Software](ctx, stmt, listOptions{
-		title:       "can't get Software",
-		activeOnly:  true,
-		skipClauses: true,
+		title:      "can't get Software",
+		activeOnly: true,
 	})
 	if err != nil {
 		return err

internal/handlers/webhooks.go

@@ -44,7 +44,7 @@ func (p *Webhook[T]) GetResourceWebhooks(ctx *fiber.Ctx) error {
 
 	stmt := p.db.Where(map[string]any{"entity_type": resource.TableName()})
 
-	return list[models.Webhook](ctx, stmt, listOptions{title: "can't get Webhooks", skipClauses: true})
+	return list[models.Webhook](ctx, stmt, listOptions{title: "can't get Webhooks", dateWindow: true})
 }
 
 // GetSingleResourceWebhooks gets the webhooks associated to a resource
@@ -65,7 +65,7 @@ func (p *Webhook[T]) GetSingleResourceWebhooks(ctx *fiber.Ctx) error {
 		Where(map[string]any{"entity_type": resource.TableName()}).
 		Where("entity_id = ?", resource.UUID())
 
-	return list[models.Webhook](ctx, stmt, listOptions{title: "can't get Webhooks", skipClauses: true})
+	return list[models.Webhook](ctx, stmt, listOptions{title: "can't get Webhooks", dateWindow: true})
 }
 
 // PostResourceWebhook creates a new webhook associated to resources

publishers_test.go

@@ -1284,6 +1284,18 @@ func TestPublishersLogsEndpoints(t *testing.T) {
 				assertOnlyKeys(t, log, "id", "createdAt", "updatedAt", "message", "entity")
 			},
 		},
+		{
+			description: `GET logs with a "from" query param later than every log`,
+			query:       "GET /v1/publishers/2ded32eb-c45e-4167-9166-a44e18b8adde/logs?from=2020-01-01T00:00:00Z",
+
+			expectedCode:        200,
+			expectedContentType: "application/json",
+			validateFunc: func(t *testing.T, response map[string]interface{}) {
+				data := assertListResponse(t, response)
+
+				assert.Empty(t, data)
+			},
+		},
 		{
 			description: "GET logs for non existing publisher",
 			query:       "GET /v1/publishers/NO_SUCH_PUBLISHER/logs",

software_test.go

@@ -1175,6 +1175,30 @@ func TestSoftwareEndpoints(t *testing.T) {
 				}
 			},
 		},
+		{
+			description: `GET logs with a "from" query param later than every log`,
+			query:       "GET /v1/software/c353756e-8597-4e46-a99b-7da2e141603b/logs?from=2020-01-01T00:00:00Z",
+
+			expectedCode:        200,
+			expectedContentType: "application/json",
+			validateFunc: func(t *testing.T, response map[string]any) {
+				data := assertListResponse(t, response)
+
+				assert.Empty(t, data)
+			},
+		},
+		{
+			description: `GET logs with a "to" query param earlier than every log`,
+			query:       "GET /v1/software/c353756e-8597-4e46-a99b-7da2e141603b/logs?to=2000-01-01T00:00:00Z",
+
+			expectedCode:        200,
+			expectedContentType: "application/json",
+			validateFunc: func(t *testing.T, response map[string]any) {
+				data := assertListResponse(t, response)
+
+				assert.Empty(t, data)
+			},
+		},
 		{
 			description: "GET logs for non existing software",
 			query:       "GET /v1/software/NO_SUCH_SOFTWARE/logs",