Added change batches, so clients can catch up on changed opportunities
Every save that stores new opportunities or new releases records them in a change batch (migration 00008: opportunity_change_batches and opportunity_change_batch_opportunities; the opportunities table is unchanged), as does every change of marks. A batch is named by a random SHA-256 hash, so it can't be guessed, and ordered by its created_at, which is kept strictly increasing even if the clock goes back. GET /api/opportunities pages now carry the newest batch, read before the page, and GET /api/opportunities?changed_after=<batch> returns every opportunity changed since that batch, unpaged, with the newest batch it includes. Offset and limit can't be combined with it, and an unknown batch is 410 Gone, telling the client to reload the whole list. OpportunitiesUpdated now carries its save's batch, and a "large" flag: over 50 opportunities, new and updated are left empty and only the summary counts them, so a big catch-up no longer sends tens of thousands of UUIDs.
This commit is contained in:
1 parent
51d6470649
commit
155b84caea
13 files changed
+699
-29
No files matched your search
@@ -24,11 +24,17 @@ var markIndex, markNames = findMarks()
|
||||
|
||||
// OpportunityPage is a page of opportunities, as GET /opportunities returns
|
||||
// it. HasMore reports that another page follows, at Offset+Limit.
|
||||
//
|
||||
// Batch is the hash of the newest change batch when the page was read, or
|
||||
// "" if none has been recorded: given as changed_after, it gets the
|
||||
// opportunities changed since. A page of changed opportunities is all of
|
||||
// them, so Limit is their number and HasMore is false.
|
||||
type OpportunityPage struct {
|
||||
Opportunities []types.Opportunity `json:"opportunities"`
|
||||
Offset int `json:"offset"`
|
||||
Limit int `json:"limit"`
|
||||
HasMore bool `json:"has_more"`
|
||||
Batch string `json:"batch"`
|
||||
}
|
||||
|
||||
// opportunityRoutes wires HTTP handlers to the opportunity store.
|
||||
@@ -65,22 +71,38 @@ func (h *opportunityRoutes) Routes(router chi.Router) {
|
||||
// list serves GET /opportunities: a page of opportunities, newest release
|
||||
// first, with their classifications and marks but not their releases. The
|
||||
// page is set by the offset and limit (default 50, at most 100) query
|
||||
// parameters.
|
||||
// parameters. With changed_after, it serves the opportunities changed since
|
||||
// that batch instead (see listChanged).
|
||||
//
|
||||
// @Summary List opportunities
|
||||
// @Description A page of opportunities, newest release first, with their
|
||||
// @Description classifications and marks but not their releases. has_more
|
||||
// @Description says whether another page follows, at offset+limit.
|
||||
// @Description says whether another page follows, at offset+limit. batch
|
||||
// @Description is the newest change batch when the page was read.
|
||||
// @Description With changed_after (a batch from a page or an
|
||||
// @Description OpportunitiesUpdated event), the page is instead every
|
||||
// @Description opportunity changed since that batch (new, given new
|
||||
// @Description releases or with changed marks), and batch is the newest
|
||||
// @Description batch they include. offset and limit can't be given with
|
||||
// @Description changed_after. An unknown batch is 410 Gone: reload the
|
||||
// @Description whole list.
|
||||
// @Tags opportunities
|
||||
// @Produce json
|
||||
// @Param offset query int false "Opportunities to skip" minimum(0) default(0)
|
||||
// @Param limit query int false "Page size" minimum(1) maximum(100) default(50)
|
||||
// @Success 200 {object} OpportunityPage
|
||||
// @Failure 400 {object} httpx.Problem "Invalid offset or limit"
|
||||
// @Failure 500 {object} httpx.Problem
|
||||
// @Param offset query int false "Opportunities to skip" minimum(0) default(0)
|
||||
// @Param limit query int false "Page size" minimum(1) maximum(100) default(50)
|
||||
// @Param changed_after query string false "List the opportunities changed after this batch instead"
|
||||
// @Success 200 {object} OpportunityPage
|
||||
// @Failure 400 {object} httpx.Problem "Invalid offset or limit, or either with changed_after"
|
||||
// @Failure 410 {object} httpx.Problem "Unknown batch"
|
||||
// @Failure 500 {object} httpx.Problem
|
||||
// @Router /opportunities [get]
|
||||
func (h *opportunityRoutes) list(w http.ResponseWriter, r *http.Request) {
|
||||
query := r.URL.Query()
|
||||
if query.Has("changed_after") {
|
||||
h.listChanged(w, r)
|
||||
return
|
||||
}
|
||||
|
||||
offset, offsetErr := intParam(query.Get("offset"), 0, 0, 1<<30)
|
||||
limit, limitErr := intParam(query.Get("limit"), 50, 1, 100)
|
||||
err := errors.Join(offsetErr, limitErr)
|
||||
@@ -89,6 +111,14 @@ func (h *opportunityRoutes) list(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// Read first, so nothing changed while the page is read is missed by
|
||||
// asking for the changes after it.
|
||||
batch, err := h.store.LatestChangeBatch(r.Context())
|
||||
if err != nil {
|
||||
httpx.WriteProblem(w, r, http.StatusInternalServerError, "")
|
||||
return
|
||||
}
|
||||
|
||||
// One more than the page, to tell whether another page follows.
|
||||
opportunities, err := h.store.ListOpportunities(
|
||||
r.Context(),
|
||||
@@ -107,6 +137,7 @@ func (h *opportunityRoutes) list(w http.ResponseWriter, r *http.Request) {
|
||||
Offset: offset,
|
||||
Limit: limit,
|
||||
HasMore: len(opportunities) > limit,
|
||||
Batch: batch,
|
||||
}
|
||||
if page.HasMore {
|
||||
page.Opportunities = opportunities[:limit]
|
||||
@@ -115,6 +146,48 @@ func (h *opportunityRoutes) list(w http.ResponseWriter, r *http.Request) {
|
||||
httpx.WriteJSON(w, http.StatusOK, page)
|
||||
}
|
||||
|
||||
// listChanged serves GET /opportunities?changed_after=batch: every
|
||||
// opportunity changed after the change batch, as one page, with the newest
|
||||
// batch it includes. An unknown batch, such as one from another database, is
|
||||
// 410 Gone, as the client can't catch up from it.
|
||||
func (h *opportunityRoutes) listChanged(
|
||||
w http.ResponseWriter, r *http.Request,
|
||||
) {
|
||||
query := r.URL.Query()
|
||||
if query.Has("offset") || query.Has("limit") {
|
||||
httpx.WriteProblem(
|
||||
w,
|
||||
r,
|
||||
http.StatusBadRequest,
|
||||
"offset and limit can't be used with changed_after",
|
||||
)
|
||||
return
|
||||
}
|
||||
|
||||
opportunities, batch, err := h.store.ListChangedOpportunities(
|
||||
r.Context(), query.Get("changed_after"),
|
||||
)
|
||||
if errors.Is(err, store.ErrUnknownBatch) {
|
||||
httpx.WriteProblem(
|
||||
w,
|
||||
r,
|
||||
http.StatusGone,
|
||||
"unknown batch: reload the whole list",
|
||||
)
|
||||
return
|
||||
}
|
||||
if err != nil {
|
||||
httpx.WriteProblem(w, r, http.StatusInternalServerError, "")
|
||||
return
|
||||
}
|
||||
|
||||
httpx.WriteJSON(w, http.StatusOK, OpportunityPage{
|
||||
Opportunities: opportunities,
|
||||
Limit: len(opportunities),
|
||||
Batch: batch,
|
||||
})
|
||||
}
|
||||
|
||||
// get serves GET /opportunities/{uuid}: one opportunity with its
|
||||
// classifications, marks and releases.
|
||||
//
|
||||
|
||||
@@ -172,6 +172,75 @@ func TestListOpportunitiesBadPaging(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A page names the newest change batch, and changed_after lists what
|
||||
// changed after a batch: here, a change of marks.
|
||||
func TestListChangedOpportunities(t *testing.T) {
|
||||
url := opportunitiesServer(t, 3)
|
||||
|
||||
var page OpportunityPage
|
||||
request(t, "GET", url+"/opportunities", &page)
|
||||
if page.Batch == "" {
|
||||
t.Fatal("the page names no batch")
|
||||
}
|
||||
|
||||
var changed OpportunityPage
|
||||
status := request(
|
||||
t, "GET", url+"/opportunities?changed_after="+page.Batch, &changed,
|
||||
)
|
||||
if status != 200 || len(changed.Opportunities) != 0 ||
|
||||
changed.Opportunities == nil || changed.Batch != page.Batch {
|
||||
t.Fatalf(
|
||||
"nothing changed: status %d, %+v; want [] and the same batch",
|
||||
status, changed,
|
||||
)
|
||||
}
|
||||
|
||||
request(t, "PUT", url+"/opportunities/ocds-2/marks/skipped", nil)
|
||||
status = request(
|
||||
t, "GET", url+"/opportunities?changed_after="+page.Batch, &changed,
|
||||
)
|
||||
if status != 200 || strings.Join(uuids(changed), ",") != "ocds-2" ||
|
||||
!changed.Opportunities[0].Marks.Skipped {
|
||||
t.Fatalf("after a mark: status %d, %+v; want ocds-2", status, changed)
|
||||
}
|
||||
if changed.Batch == page.Batch || changed.Limit != 1 || changed.HasMore {
|
||||
t.Errorf(
|
||||
"after a mark: batch %q, limit %d, has_more %v; "+
|
||||
"want a newer batch, 1, false",
|
||||
changed.Batch, changed.Limit, changed.HasMore,
|
||||
)
|
||||
}
|
||||
|
||||
var latest OpportunityPage
|
||||
request(t, "GET", url+"/opportunities?limit=1", &latest)
|
||||
if latest.Batch != changed.Batch {
|
||||
t.Errorf("page batch %q; want %q", latest.Batch, changed.Batch)
|
||||
}
|
||||
}
|
||||
|
||||
// An unknown batch is 410 Gone, and paging can't be used with changed_after.
|
||||
func TestListChangedOpportunitiesRefused(t *testing.T) {
|
||||
url := opportunitiesServer(t, 1)
|
||||
var page OpportunityPage
|
||||
request(t, "GET", url+"/opportunities", &page)
|
||||
|
||||
tests := []struct {
|
||||
query string
|
||||
want int
|
||||
}{
|
||||
{"?changed_after=unknown", http.StatusGone},
|
||||
{"?changed_after=", http.StatusGone},
|
||||
{"?changed_after=" + page.Batch + "&limit=5", http.StatusBadRequest},
|
||||
{"?offset=0&changed_after=" + page.Batch, http.StatusBadRequest},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
status := request(t, "GET", url+"/opportunities"+tt.query, nil)
|
||||
if status != tt.want {
|
||||
t.Errorf("%s: status %d; want %d", tt.query, status, tt.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// One opportunity comes with its releases; a missing one is 404.
|
||||
func TestGetOpportunity(t *testing.T) {
|
||||
url := opportunitiesServer(t, 2)
|
||||
|
||||
Reference in new issue
Block a user