tree f016c64c0922b8a9e2d9ed7fb93e80ed1bceaae9
parent 1673f18a424f1bbe431af59cb2e472b701033692
author Patrick Hiesel <hiesel@google.com> 1570722934 +0200
committer Patrick Hiesel <hiesel@google.com> 1570784059 +0200

Fix a bug where _moreChanges is set to the wrong value

The query changes REST endpoint supports sending multiple queries in a
single request. The backend implementation uses a cache (a simple map)
to spare computing a ChangeInfo object more than once in case it shows
up in more than a single query result.

The implementation contained a bug that would set _moreChanges on the
last entity of one query that has more results than get sent. However,
through the reuse of the entity, this boolean would also end up in other
entities, where it's simply wrong.

There are multiple, alternative ways how this can be fixed:
1) Remove the cache: We could simply remove the map. This has an unknown
   performance impact.
2) Deep-copy objects instead of plainly reusing them: This would add
   boiler-plate to a lot of *Info objects that ChangeInfo depends on.
   Hopefully we can consider moving to immutable API objects (AutoValue
   or Protobuf) in the future, which would obsolete this code. It still
   seems undesirable to add this now. Notably, ActionJson already
   contains what looks like brittle copy logic.
3) Rework the JSON response to be a List<QueryResponse> where
   QueryResponse is an object that has List<ChangeInfo> and _more: Would
   be the right path design-wise, but is a non-starter because it's a
   major change to a heavily used API.
4) Never reuse the last entity in a result set: In this way, the last
   entity is always unique. Setting _moreChanges on it has no side
   effects.

This commit implements (4), which seems like the least evil.

Change-Id: I6b007d442249d0445ea49f125c45d8f9022e104e