Only set an ETag for the notes endpoint after all notes have been sent
What does this MR do?
The ETag caching system we use assumes that all notes are sent in a single response, and makes that response the cached one. When a note is subsequently updated, the ETag is invalidated and the note thrown away.
This is incompatible with pagination, which is currently behind a flag (appropriately, called paginated_notes. I think it also means that the clients end up parsing the cached response every poll-interval, which is less than ideal.
One approach is to remove the ETag from this endpoint altogether, but this has some costs in terms of additional server load. The MR for that is here: !42226 (closed)
In this MR, I introduce a mechanism to make the ETag conditional, and skip adding the ETag if there are any notes in the response. This means that:
- Pagination will work correctly
- The browser will only ever cache the empty response
This means there will be one additional HTTP request before the ETag is applied, but I think it's overall positive in its own right, and allowing paginated notes to work is a big plus too.
Screenshots (strongly suggested)
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec -
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Related to #209784 (closed)