[UID2-2005] Add new endpoint to check opt out status by raw UIDs #557
Conversation
This reverts commit d8e563a.
| @@ -168,6 +173,8 @@ public UIDOperatorVerticle(JsonObject config, | |||
| this.allowClockSkewSeconds = config.getInteger(Const.Config.AllowClockSkewSecondsProp, 1800); | |||
| this.maxSharingLifetimeSeconds = config.getInteger(Const.Config.MaxSharingLifetimeProp, config.getInteger(Const.Config.SharingTokenExpiryProp)); | |||
| this.saltRetrievalResponseHandler = saltRetrievalResponseHandler; | |||
| this.optOutStatusApiEnabled = config.getBoolean(Const.Config.OptOutStatusApiEnabled, false); | |||
| this.optOutStatusMaxRequestSize = config.getInteger(Const.Config.OptOutStatusMaxRequestSize, 1000); | |||
There was a problem hiding this comment.
the Optout API confluence page stated 5000? any reason why default stays at 1000?
There was a problem hiding this comment.
I suggested we do some benchmarking and see if 1,000 would be a better place to start than 5,000, since can we always increase the limit later.
I've just benchmarked it and I think we're safe to go with 5,000. @asloobq could you please update the default?
| @ParameterizedTest | ||
| @MethodSource("optOutStatusRequestData") | ||
| void optOutStatusRequest(Map<String, Long> optedOutIds, int optedOutCount, Vertx vertx, VertxTestContext testContext) { | ||
| fakeAuth(126, Role.MAPPER); |
There was a problem hiding this comment.
could you parameterise the role to test with id_reader/sharer too?
| @@ -144,8 +166,9 @@ private byte[] entriesToByteArray(List<OptOutEntry> entries) { | |||
| return bytes; | |||
| } | |||
|
|
|||
| private JsonObject make1mOptOutEntryConfig() { | |||
| private JsonObject make1mOptOutEntryConfig(boolean isEnabled) { | |||
There was a problem hiding this comment.
maybe rename to OptOutStatusApiEnabled as isEnabled is not clear on what is enabled, optout storage/optout lookup/or optout api?
| @@ -79,6 +79,11 @@ public Instant getLatestEntry(UserIdentity firstLevelHashIdentity) { | |||
| return instant; | |||
| } | |||
|
|
|||
| @Override | |||
| public long getLatestEntryByAdId(String adId) { | |||
There was a problem hiding this comment.
We actually return the earliest timestamp. Can we rename to getAdIdOptOutTimestamp or getOptOutTimestampByAdId?
| @@ -168,6 +173,8 @@ public UIDOperatorVerticle(JsonObject config, | |||
| this.allowClockSkewSeconds = config.getInteger(Const.Config.AllowClockSkewSecondsProp, 1800); | |||
| this.maxSharingLifetimeSeconds = config.getInteger(Const.Config.MaxSharingLifetimeProp, config.getInteger(Const.Config.SharingTokenExpiryProp)); | |||
| this.saltRetrievalResponseHandler = saltRetrievalResponseHandler; | |||
| this.optOutStatusApiEnabled = config.getBoolean(Const.Config.OptOutStatusApiEnabled, false); | |||
| this.optOutStatusMaxRequestSize = config.getInteger(Const.Config.OptOutStatusMaxRequestSize, 1000); | |||
There was a problem hiding this comment.
I suggested we do some benchmarking and see if 1,000 would be a better place to start than 5,000, since can we always increase the limit later.
I've just benchmarked it and I think we're safe to go with 5,000. @asloobq could you please update the default?
|
|
||
| private static Stream<Arguments> optOutStatusValidationErrorData() { | ||
| // Test case 1 | ||
| int requestLimit = 1000; |
There was a problem hiding this comment.
At the moment if someone changes the default value in the verticle, they will have to run the tests in order to find that they need to change the value here as well.
Can we make this a field of the test class and use it to configure the verticle, so that changes to the default value in the verticle won't cause this test to break?
* Add new endpoint to check opt out status by raw UIDs * Try loading optout deltas from local * Revert "Try loading optout deltas from local" This reverts commit d8e563a. * Add a switch to disable endpoint and hashmap loading * Add Snapshot store test for disabled status * Add test cases for optout status endpoint processing * Increase max request size to 5K. Refactor and update tests * Update opt out status test * Update default max size of opt out request
Manual Testing:
Automated Testing