From 0fc026173aa9c75e74697193cd20acc2f8b3baa5 Mon Sep 17 00:00:00 2001 From: petruki <31597636+petruki@users.noreply.github.com> Date: Sat, 12 Aug 2023 20:02:58 -0700 Subject: [PATCH 1/2] Closes #249 - improved isTokenValid performance --- .../client/remote/ClientWSImpl.java | 30 +++-- .../service/remote/ClientRemoteService.java | 125 +++++++++++------- .../client/SwitcherApiMockTest.java | 9 -- .../SwitcherSnapshotAutoUpdateTest.java | 9 +- 4 files changed, 97 insertions(+), 76 deletions(-) diff --git a/src/main/java/com/github/switcherapi/client/remote/ClientWSImpl.java b/src/main/java/com/github/switcherapi/client/remote/ClientWSImpl.java index 37043f4f..f24ee81e 100644 --- a/src/main/java/com/github/switcherapi/client/remote/ClientWSImpl.java +++ b/src/main/java/com/github/switcherapi/client/remote/ClientWSImpl.java @@ -59,21 +59,25 @@ public CriteriaResponse executeCriteriaService(final Switcher switcher, final St .queryParam(Switcher.KEY, switcher.getSwitcherKey()) .queryParam(Switcher.SHOW_REASON, switcher.isShowReason()) .queryParam(Switcher.BYPASS_METRIC, switcher.isBypassMetrics()); - - final Response response = myResource.request(MediaType.APPLICATION_JSON) - .header(HEADER_AUTHORIZATION, String.format(TOKEN_TEXT, token)) - .post(Entity.json(switcher.getInputRequest())); - if (response.getStatus() == 200) { - final CriteriaResponse criteriaResponse = response.readEntity(CriteriaResponse.class); - criteriaResponse.setSwitcherKey(switcher.getSwitcherKey()); - criteriaResponse.setEntry(switcher.getEntry()); - response.close(); + try { + final Response response = myResource.request(MediaType.APPLICATION_JSON) + .header(HEADER_AUTHORIZATION, String.format(TOKEN_TEXT, token)) + .post(Entity.json(switcher.getInputRequest())); - return criteriaResponse; - } + if (response.getStatus() == 200) { + final CriteriaResponse criteriaResponse = response.readEntity(CriteriaResponse.class); + criteriaResponse.setSwitcherKey(switcher.getSwitcherKey()); + criteriaResponse.setEntry(switcher.getEntry()); + response.close(); - throw new SwitcherRemoteException(url, response.getStatus()); + return criteriaResponse; + } + + throw new SwitcherRemoteException(url, response.getStatus()); + } catch (Exception e) { + throw new SwitcherRemoteException(url, e); + } } @Override @@ -85,7 +89,7 @@ public Optional auth() { final String url = SwitcherContextBase.contextStr(ContextKey.URL); final WebTarget myResource = client.target(String.format(AUTH_URL, url)); - + final Response response = myResource.request(MediaType.APPLICATION_JSON) .header(HEADER_APIKEY, SwitcherContextBase.contextStr(ContextKey.APIKEY)) .post(Entity.json(authRequest)); diff --git a/src/main/java/com/github/switcherapi/client/service/remote/ClientRemoteService.java b/src/main/java/com/github/switcherapi/client/service/remote/ClientRemoteService.java index e627ae2d..19028b7c 100644 --- a/src/main/java/com/github/switcherapi/client/service/remote/ClientRemoteService.java +++ b/src/main/java/com/github/switcherapi/client/service/remote/ClientRemoteService.java @@ -1,13 +1,9 @@ package com.github.switcherapi.client.service.remote; -import java.util.Date; -import java.util.Optional; -import java.util.Set; - import com.github.switcherapi.client.SwitcherContextBase; -import com.github.switcherapi.client.exception.SwitcherRemoteException; import com.github.switcherapi.client.exception.SwitcherException; import com.github.switcherapi.client.exception.SwitcherInvalidDateTimeArgumentException; +import com.github.switcherapi.client.exception.SwitcherRemoteException; import com.github.switcherapi.client.model.ContextKey; import com.github.switcherapi.client.model.Switcher; import com.github.switcherapi.client.model.criteria.Snapshot; @@ -19,74 +15,111 @@ import com.github.switcherapi.client.remote.ClientWSImpl; import com.github.switcherapi.client.utils.SwitcherUtils; +import java.util.Date; +import java.util.Optional; +import java.util.Set; + /** * @author Roger Floriano (petruki) * @since 2019-12-24 */ public class ClientRemoteService { - private static ClientRemoteService instance; - private final ClientWS clientWs; - private Optional authResponse = Optional.empty(); + private AuthResponse authResponse; + + private enum TokenStatus { + VALID, INVALID, SILENT + } + + private enum Singleton { + INSTANCE; + + private final ClientRemoteService instance = new ClientRemoteService(); + + public ClientRemoteService getInstance() { + return this.instance; + } + } private ClientRemoteService() { this.clientWs = new ClientWSImpl(); } public static ClientRemoteService getInstance() { - if (instance == null) { - instance = new ClientRemoteService(); - } - return instance; + return Singleton.INSTANCE.getInstance(); } public CriteriaResponse executeCriteria(final Switcher switcher) { - if (!this.isTokenValid()) { - this.auth(); + final TokenStatus tokenStatus = this.isTokenValid(); + + try { + if (tokenStatus == TokenStatus.INVALID) { + this.auth(); + } + + if (tokenStatus == TokenStatus.SILENT) { + throw new SwitcherRemoteException(SwitcherContextBase.contextStr(ContextKey.URL)); + } + + return this.clientWs.executeCriteriaService(switcher, + Optional.of(this.authResponse).orElseGet(AuthResponse::new).getToken()); + } catch (final SwitcherRemoteException e) { + if (tokenStatus != TokenStatus.SILENT) { + this.setSilentModeExpiration(); + } + + throw e; } - - return this.clientWs.executeCriteriaService( - switcher, this.authResponse.get().getToken()); } public Snapshot resolveSnapshot() throws SwitcherException { - if (!this.isTokenValid()) { + if (this.isTokenValid() == TokenStatus.INVALID) { this.auth(); } return this.clientWs.resolveSnapshot( - this.authResponse.orElseGet(AuthResponse::new).getToken()); + Optional.of(this.authResponse).orElseGet(AuthResponse::new).getToken()); } public boolean checkSnapshotVersion(final long version) { - if (!this.isTokenValid()) { + if (this.isTokenValid() == TokenStatus.INVALID) { this.auth(); } - final SnapshotVersionResponse snapshotVersionResponse = this.clientWs.checkSnapshotVersion(version, - this.authResponse.orElseGet(AuthResponse::new).getToken()); + final SnapshotVersionResponse snapshotVersionResponse = this.clientWs.checkSnapshotVersion(version, + Optional.of(this.authResponse).orElseGet(AuthResponse::new).getToken()); return snapshotVersionResponse.isUpdated(); } public SwitchersCheck checkSwitchers(final Set switchers) { + final TokenStatus tokenStatus = this.isTokenValid(); + try { - if (!this.isTokenValid()) { + if (tokenStatus == TokenStatus.INVALID) { this.auth(); } - - return this.clientWs.checkSwitchers(switchers, - this.authResponse.orElseGet(AuthResponse::new).getToken()); - } catch (final Exception e) { - throw new SwitcherRemoteException(SwitcherContextBase.contextStr(ContextKey.URL), e); + + if (tokenStatus == TokenStatus.SILENT) { + throw new SwitcherRemoteException(SwitcherContextBase.contextStr(ContextKey.URL)); + } + + return this.clientWs.checkSwitchers(switchers, + Optional.of(this.authResponse).orElseGet(AuthResponse::new).getToken()); + } catch (final SwitcherRemoteException e) { + if (tokenStatus != TokenStatus.SILENT) { + this.setSilentModeExpiration(); + } + + throw e; } } private void auth() { try { - this.authResponse = this.clientWs.auth(); + this.authResponse = this.clientWs.auth().orElseGet(AuthResponse::new); } catch (final SwitcherException e) { throw e; } catch (final Exception e) { @@ -95,24 +128,22 @@ private void auth() { } } - private boolean isTokenValid() throws SwitcherRemoteException, + private TokenStatus isTokenValid() throws SwitcherRemoteException, SwitcherInvalidDateTimeArgumentException { - - if (this.authResponse.isPresent()) { - if (this.authResponse.get().getToken().equals(ContextKey.SILENT_MODE.getParam()) - && !this.authResponse.get().isExpired()) { - throw new SwitcherRemoteException(SwitcherContextBase.contextStr(ContextKey.URL)); - } else { - if (!this.clientWs.isAlive()) { - this.setSilentModeExpiration(); - throw new SwitcherRemoteException(SwitcherContextBase.contextStr(ContextKey.URL)); - } - - return !this.authResponse.orElseGet(AuthResponse::new).isExpired(); - } + + final Optional optAuthResponse = Optional.ofNullable(this.authResponse); + + if (optAuthResponse.isEmpty()) { + return TokenStatus.INVALID; } - - return false; + + if (optAuthResponse.get().getToken().equals(ContextKey.SILENT_MODE.getParam()) + && !optAuthResponse.get().isExpired()) { + return TokenStatus.SILENT; + } + + return optAuthResponse.orElseGet(AuthResponse::new).isExpired() ? + TokenStatus.INVALID : TokenStatus.VALID; } private void setSilentModeExpiration() throws SwitcherInvalidDateTimeArgumentException { @@ -122,12 +153,12 @@ private void setSilentModeExpiration() throws SwitcherInvalidDateTimeArgumentExc response.setToken(ContextKey.SILENT_MODE.getParam()); response.setExp(SwitcherUtils.addTimeDuration(addValue, new Date()).getTime()/1000); - this.authResponse = Optional.of(response); + this.authResponse = response; } } public void clearAuthResponse() { - this.authResponse = Optional.empty(); + this.authResponse = null; } } diff --git a/src/test/java/com/github/switcherapi/client/SwitcherApiMockTest.java b/src/test/java/com/github/switcherapi/client/SwitcherApiMockTest.java index d3264eb1..ca9245e0 100644 --- a/src/test/java/com/github/switcherapi/client/SwitcherApiMockTest.java +++ b/src/test/java/com/github/switcherapi/client/SwitcherApiMockTest.java @@ -317,9 +317,6 @@ void shouldReturnTrue_tokenExpired() throws InterruptedException { CountDownLatch waiter = new CountDownLatch(1); waiter.await(2, TimeUnit.SECONDS); - //isAlive - givenResponse(generateStatusResponse("200")); - //auth givenResponse(generateMockAuth(2)); @@ -338,9 +335,6 @@ void shouldValidateAndUpdateSnapshot() { //criteria/snapshot_check givenResponse(generateCheckSnapshotVersionResponse("false")); - //auth isAlive - givenResponse(generateMockAuth(10)); - //graphql givenResponse(generateSnapshotResponse()); @@ -438,9 +432,6 @@ void shouldValidateAndLoadSnapshot_whenOffline() { //criteria/snapshot_check givenResponse(generateCheckSnapshotVersionResponse("false")); - //auth isAlive - givenResponse(generateMockAuth(10)); - //graphql givenResponse(generateSnapshotResponse()); diff --git a/src/test/java/com/github/switcherapi/client/SwitcherSnapshotAutoUpdateTest.java b/src/test/java/com/github/switcherapi/client/SwitcherSnapshotAutoUpdateTest.java index 0aae3b08..994324d7 100644 --- a/src/test/java/com/github/switcherapi/client/SwitcherSnapshotAutoUpdateTest.java +++ b/src/test/java/com/github/switcherapi/client/SwitcherSnapshotAutoUpdateTest.java @@ -127,9 +127,6 @@ private void givenSnapshotUpdateResponse(boolean isUpdated) { givenResponse(generateCheckSnapshotVersionResponse(Boolean.toString(isUpdated))); if (!isUpdated) { - //auth isAlive - givenResponse(generateMockAuth()); - //graphql givenResponse(generateSnapshotResponse("default.json")); } @@ -215,12 +212,10 @@ void shouldNotUpdateSnapshot_whenNoUpdateAvailable() throws InterruptedException @Order(4) void shouldUpdateSnapshot_online_inMemory() throws InterruptedException { //given - //load snapshot in-memory givenResponse(generateMockAuth()); //auth givenResponse(generateSnapshotResponse("default_outdated.json")); //graphql - - //update snapshot - givenSnapshotUpdateResponse(false); + givenResponse(generateCheckSnapshotVersionResponse(Boolean.toString(false))); //criteria/snapshot_check + givenResponse(generateSnapshotResponse("default.json")); //graphql //that Switchers.configure(ContextBuilder.builder() From 0354d6d862746880a4a5a266c6cb7e3e4b883693 Mon Sep 17 00:00:00 2001 From: petruki <31597636+petruki@users.noreply.github.com> Date: Sat, 12 Aug 2023 20:38:23 -0700 Subject: [PATCH 2/2] Improved auth logic --- .../client/remote/ClientWSImpl.java | 22 ++++++---- .../service/remote/ClientRemoteService.java | 43 ++++++------------- 2 files changed, 26 insertions(+), 39 deletions(-) diff --git a/src/main/java/com/github/switcherapi/client/remote/ClientWSImpl.java b/src/main/java/com/github/switcherapi/client/remote/ClientWSImpl.java index f24ee81e..c5487c3d 100644 --- a/src/main/java/com/github/switcherapi/client/remote/ClientWSImpl.java +++ b/src/main/java/com/github/switcherapi/client/remote/ClientWSImpl.java @@ -90,18 +90,22 @@ public Optional auth() { final String url = SwitcherContextBase.contextStr(ContextKey.URL); final WebTarget myResource = client.target(String.format(AUTH_URL, url)); - final Response response = myResource.request(MediaType.APPLICATION_JSON) - .header(HEADER_APIKEY, SwitcherContextBase.contextStr(ContextKey.APIKEY)) - .post(Entity.json(authRequest)); + try { + final Response response = myResource.request(MediaType.APPLICATION_JSON) + .header(HEADER_APIKEY, SwitcherContextBase.contextStr(ContextKey.APIKEY)) + .post(Entity.json(authRequest)); - if (response.getStatus() == 200) { - Optional authResponse = Optional.of(response.readEntity(AuthResponse.class)); - response.close(); + if (response.getStatus() == 200) { + Optional authResponse = Optional.of(response.readEntity(AuthResponse.class)); + response.close(); - return authResponse; - } + return authResponse; + } - throw new SwitcherRemoteException(url, response.getStatus()); + throw new SwitcherRemoteException(url, response.getStatus()); + } catch (Exception e) { + throw new SwitcherRemoteException(url, e); + } } @Override diff --git a/src/main/java/com/github/switcherapi/client/service/remote/ClientRemoteService.java b/src/main/java/com/github/switcherapi/client/service/remote/ClientRemoteService.java index 19028b7c..0c7b81c2 100644 --- a/src/main/java/com/github/switcherapi/client/service/remote/ClientRemoteService.java +++ b/src/main/java/com/github/switcherapi/client/service/remote/ClientRemoteService.java @@ -55,13 +55,7 @@ public CriteriaResponse executeCriteria(final Switcher switcher) { final TokenStatus tokenStatus = this.isTokenValid(); try { - if (tokenStatus == TokenStatus.INVALID) { - this.auth(); - } - - if (tokenStatus == TokenStatus.SILENT) { - throw new SwitcherRemoteException(SwitcherContextBase.contextStr(ContextKey.URL)); - } + this.auth(tokenStatus); return this.clientWs.executeCriteriaService(switcher, Optional.of(this.authResponse).orElseGet(AuthResponse::new).getToken()); @@ -73,21 +67,17 @@ public CriteriaResponse executeCriteria(final Switcher switcher) { throw e; } } - + public Snapshot resolveSnapshot() throws SwitcherException { - if (this.isTokenValid() == TokenStatus.INVALID) { - this.auth(); - } + this.auth(this.isTokenValid()); return this.clientWs.resolveSnapshot( Optional.of(this.authResponse).orElseGet(AuthResponse::new).getToken()); } public boolean checkSnapshotVersion(final long version) { - if (this.isTokenValid() == TokenStatus.INVALID) { - this.auth(); - } - + this.auth(this.isTokenValid()); + final SnapshotVersionResponse snapshotVersionResponse = this.clientWs.checkSnapshotVersion(version, Optional.of(this.authResponse).orElseGet(AuthResponse::new).getToken()); @@ -98,13 +88,7 @@ public SwitchersCheck checkSwitchers(final Set switchers) { final TokenStatus tokenStatus = this.isTokenValid(); try { - if (tokenStatus == TokenStatus.INVALID) { - this.auth(); - } - - if (tokenStatus == TokenStatus.SILENT) { - throw new SwitcherRemoteException(SwitcherContextBase.contextStr(ContextKey.URL)); - } + this.auth(tokenStatus); return this.clientWs.checkSwitchers(switchers, Optional.of(this.authResponse).orElseGet(AuthResponse::new).getToken()); @@ -116,15 +100,14 @@ public SwitchersCheck checkSwitchers(final Set switchers) { throw e; } } - - private void auth() { - try { + + private void auth(TokenStatus tokenStatus) { + if (tokenStatus == TokenStatus.INVALID) { this.authResponse = this.clientWs.auth().orElseGet(AuthResponse::new); - } catch (final SwitcherException e) { - throw e; - } catch (final Exception e) { - this.setSilentModeExpiration(); - throw new SwitcherRemoteException(SwitcherContextBase.contextStr(ContextKey.URL), e); + } + + if (tokenStatus == TokenStatus.SILENT) { + throw new SwitcherRemoteException(SwitcherContextBase.contextStr(ContextKey.URL)); } }