Skip to content

Fast eventcounter etags for geoextracts #1657

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Fast eventcounter etags for geoextracts #1657

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f33fd5
allow for generation of weak etags
brontolosone Oct 26, 2025
b56d56c
eventcounter-etag-based revalidation for geodata endpoints
brontolosone Oct 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/model/query/actees.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,9 @@ const provision = (species, parent) => ({ one }) =>
one(sql`insert into actees (id, species, parent) values (${uuid()}, ${species}, ${(parent == null) ? null : parent.acteeId}) returning *`)
.then(construct(Actee));

module.exports = { provision };
const getEventCount = (acteeId) => ({ oneFirst }) => oneFirst(sql`
SELECT coalesce(sum(evt_count), 0) FROM eventcounters WHERE "acteeId" = ${acteeId}
`);

module.exports = { provision, getEventCount };

21 changes: 16 additions & 5 deletions lib/resources/geo-extracts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
const { getOrNotFound } = require('../util/promise');
const { Form } = require('../model/frames');
const { Sanitize } = require('../util/param-sanitize');
const { isTrue, json } = require('../util/http');
const { isTrue, json, withEtag } = require('../util/http');
const Problem = require('../util/problem');


Expand All @@ -37,13 +37,15 @@ module.exports = (service, endpoint) => {


// Bulk endpoints
service.get('/projects/:projectId/forms/:xmlFormId/submissions.geojson', endpoint.plain(async ({ Forms, GeoExtracts }, { auth, query, params }) => {
service.get('/projects/:projectId/forms/:xmlFormId/submissions.geojson', endpoint.plain(async ({ Forms, GeoExtracts, Actees }, { auth, query, params }) => {

const form = await Forms.getByProjectAndXmlFormId(params.projectId, params.xmlFormId, Form.WithoutDef, Form.WithoutXml)
.then(getOrNotFound)
.then((foundForm) => auth.canOrReject('submission.list', foundForm));

return GeoExtracts.getSubmissionFeatureCollectionGeoJson(
const acteeVersion = await Actees.getEventCount(form.acteeId);

const createResponse = () => GeoExtracts.getSubmissionFeatureCollectionGeoJson(
form.id,
Sanitize.queryParamToArray(query.submissionID),
Sanitize.queryParamToArray(query.fieldpath),
Expand All @@ -53,15 +55,21 @@ module.exports = (service, endpoint) => {
isTrue(query.deleted),
Number.parseInt(query.limit, 10) || null,
).then(json);

// Weak etag, as the order in the resultset is undefined.
return withEtag(acteeVersion, createResponse, true);
}));

service.get('/projects/:projectId/datasets/:datasetName/entities.geojson', endpoint.plain(async ({ Datasets, GeoExtracts }, { auth, query, params }) => {

service.get('/projects/:projectId/datasets/:datasetName/entities.geojson', endpoint.plain(async ({ Datasets, GeoExtracts, Actees }, { auth, query, params }) => {

const foundDataset = await Datasets.get(params.projectId, params.datasetName, true)
.then(getOrNotFound)
.then((dataset) => auth.canOrReject('entity.list', dataset));

return GeoExtracts.getEntityFeatureCollectionGeoJson(
const acteeVersion = await Actees.getEventCount(foundDataset.acteeId);

const createResponse = () => GeoExtracts.getEntityFeatureCollectionGeoJson(
foundDataset.id,
Sanitize.queryParamToUuidArray(query.entityUUID, 'entityUUID'),
Sanitize.queryParamToIntArray(query.creatorId, 'creatorId'),
Expand All @@ -70,6 +78,9 @@ module.exports = (service, endpoint) => {
isTrue(query.deleted),
Number.parseInt(query.limit, 10) || null,
).then(json);

// Weak etag, as the order in the resultset is undefined.
return withEtag(acteeVersion, createResponse, true);
Copy link
Member

@matthew-white matthew-white Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it expected that the ETag doesn't change when query parameters change? I think so, just wanted to check.

Copy link
Contributor

@alxndrsn alxndrsn Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 subscribing to this query

Copy link
Contributor Author

@brontolosone brontolosone Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very much expected, yes. In HTTP caching semantics, (with a few caveats, most prominently having to do with the Vary response header), the identity of a resource is the URL, including the query parameters.

If a browser sets an If-None-Match on a request for resource with identity A to the ETag received in an earlier response for a resource with identity B, then that'd be a serious bug (in the browser) ;-)

The fact that query parameters are part of the resource identity is actually even exploited for certain so called "cache busting" approaches.

The downside is that the order of parameters in the query matters. So two URLs that effectively deliver the same data, as they have the same meaning for the application (eg ?offset=10&limit=20 vs ?limit=20&offset=10) are distinct resources to HTTP caches, and they don't reuse the cache of the one for the other. They fortunately don't as they absolutely shouldn't, because they don't know the application semantics!
I would be well within my rights to write an application that does something completely different for ?a=1&b=2 vs ?b=2&a=1, and HTTP caching should still work.

Copy link
Contributor Author

@brontolosone brontolosone Oct 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(quoting myself)

The downside is that the order of parameters in the query matters. So two URLs that effectively deliver the same data, as they have the same meaning for the application (eg ?offset=10&limit=20 vs ?limit=20&offset=10) are distinct resources to HTTP caches, and they don't reuse the cache of the one for the other. They fortunately don't as they absolutely shouldn't, because they don't know the application semantics!

So, to expand on that a bit, corollary:

If you want share a cache of computation results between requests to ?offset=10&limit=20 and ?limit=20&offset=10, you can't really do that with HTTP caching semantics. Intermediate caching proxies don't want to presume that these are effectively the same to your application, and there's no way to tell them (or indeed the browser cache) otherwise.†
The component best situated to understand the application semantics is... the application! surprise!
So, when there is a desire to share a cache between ?offset=10&limit=20 and ?limit=20&offset=10, one would do (potentially additional) caching at the application. For us that'd mean we would, all "from nodejs", compute the result, come up with a caching key (a component of which in this case would be a normalized form of the query parameters), and then store the result in something like redis or memcached (or even just plain postgresql, or files in the filesystem). That's quite a common setup!

†) Although, nginx accommodates "bring your own caching key" setups. But that's in a reverse proxy role where you have knowledge of the application semantics.
Forward caching HTTP proxies such as Squid (largely outmoded because everything has become E2E TLS in the last 10-15 years) can maybe not even be configured to bend the identity rules.

Copy link
Contributor

@alxndrsn alxndrsn Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In HTTP caching semantics, (with a few caveats, most prominently having to do with the Vary response header), the identity of a resource is the URL, including the query parameters.

👍 makes sense to me

Copy link
Member

@matthew-white matthew-white Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, this makes sense to me.

I don't think we need to do anything special at this point related to the order of query parameters.

}));

};
4 changes: 2 additions & 2 deletions lib/util/http.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,9 @@ const url = (strings, ...parts) => {

// Checks Etag in the request if it matches serverEtag then returns 304
// Otherwise executes given function 'fn'
const withEtag = (serverEtag, fn) => (request, response) => {
const withEtag = (serverEtag, fn, isWeak=false) => (request, response) => {

response.set('ETag', `"${serverEtag}"`);
response.set('ETag', `${isWeak ? 'W/': ''}"${serverEtag}"`);

// Etag logic inspired from https://stackoverflow.com/questions/72334843/custom-computed-etag-for-express-js/72335674#72335674
const clientEtag = request.get('If-None-Match');
Expand Down
Loading