Add Implementation for ListKeyVersions Api and AbstractKVStore tests for the same #8
Conversation
app/src/main/java/org/vss/impl/postgres/PostgresBackendImpl.java
Outdated
Show resolved
Hide resolved
| .getResult().map(record -> record.into(VssDbRecord.class)); | ||
|
|
||
| List<KeyValue> keyVersions = vssDbRecords.stream() | ||
| .filter(kv -> !GLOBAL_VERSION_KEY.equals(kv.getKey())) |
There was a problem hiding this comment.
Would this mean the number of entries may be one less than pageSize even though there are pageSize matches when the key prefix is empty? Should we filter at the SQL level instead?
There was a problem hiding this comment.
Yes, it can mean number of entries in response can be one less than pageSize.
But that shouldn't be a concern and client should not assume response to contain specific number of entries.
For e.g. max number of results in paginated response can change at anytime with no notice to client.
We already caution against this in api doc in proto:
"Caution: Clients must not assume a specific number of key_versions to be present in a page for paginated response."
Only way to know whether nextPage exists or not is by presence of nextPageToken.
There was a problem hiding this comment.
That doesn't answer my second question. :)
Should we filter at the SQL level instead?
Is there a reason not to?
There was a problem hiding this comment.
There is no big reason in case of SQL, i can do that. (apart from client/api-expectation and precedent)
It is just something to keep in mind that this operation might not be supported by all KV-database i.e. (list along with key not equals).
There was a problem hiding this comment.
That's ok. Feel fee to leave it as is.
app/src/main/java/org/vss/impl/postgres/PostgresBackendImpl.java
Outdated
Show resolved
Hide resolved
|
Could you rebase? |
G8XSU
force-pushed
the
keys-summary-impl
branch
from
1b2cd0b to
97b3b83
Compare
|
Done |
| globalVersion = get(GetObjectRequest.newBuilder() | ||
| GetObjectRequest getGlobalVersionRequest = GetObjectRequest.newBuilder() | ||
| .setStoreId(storeId) | ||
| .setKey(GLOBAL_VERSION_KEY) | ||
| .build()).getValue().getVersion(); | ||
| .build(); | ||
| globalVersion = get(getGlobalVersionRequest).getValue().getVersion(); |
There was a problem hiding this comment.
Changes here a below belong on the first commit.
G8XSU
force-pushed
the
keys-summary-impl
branch
from
97b3b83 to
69fb119
Compare
Depends on #5 and #6