From 343e5621d6994d4576a2de08e5ec9c9a7690e049 Mon Sep 17 00:00:00 2001 From: Aymeric Date: Thu, 22 Sep 2022 18:44:33 +0300 Subject: [PATCH] Ignore 429s when validating a connection (#83) --- .github/workflows/deploy-javadoc.yml | 3 ++- build.gradle | 10 ++++----- .../jdbc/connection/FireboltConnection.java | 19 +++++++++++------ .../jdbc/exception/ExceptionType.java | 2 +- .../jdbc/exception/FireboltException.java | 2 ++ .../connection/FireboltConnectionTest.java | 21 ++++++++++++++++++- 6 files changed, 43 insertions(+), 14 deletions(-) diff --git a/.github/workflows/deploy-javadoc.yml b/.github/workflows/deploy-javadoc.yml index 3ff033632..4664f3068 100644 --- a/.github/workflows/deploy-javadoc.yml +++ b/.github/workflows/deploy-javadoc.yml @@ -3,9 +3,10 @@ name: Deploy Javadoc on: workflow_dispatch: release: - types: [published] + types: [ published ] jobs: publish: + if: ${{ !contains(github.event.release.name, 'SNAPSHOT') }} runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 diff --git a/build.gradle b/build.gradle index eae46b1dd..f783e7453 100644 --- a/build.gradle +++ b/build.gradle @@ -9,7 +9,7 @@ plugins { } group 'com.firebolt' -version = '2.1.1-SNAPSHOT' +version = '2.1.2-SNAPSHOT' repositories { mavenCentral() @@ -55,7 +55,7 @@ sourceSets { dependencies { implementation 'org.apache.commons:commons-lang3:3.12.0' implementation 'com.google.guava:guava:31.1-jre' - implementation 'com.fasterxml.jackson.core:jackson-databind:2.13.3' + implementation 'com.fasterxml.jackson.core:jackson-databind:2.13.4' implementation 'commons-codec:commons-codec:1.15' implementation 'net.jodah:expiringmap:0.5.10' implementation 'org.apache.httpcomponents.client5:httpclient5:5.1.3' @@ -73,9 +73,9 @@ dependencies { testCompileOnly 'org.projectlombok:lombok:1.18.24' testAnnotationProcessor 'org.projectlombok:lombok:1.18.24' - testImplementation 'org.mockito:mockito-junit-jupiter:4.7.0' - testImplementation 'org.mockito:mockito-core:4.7.0' - testImplementation 'org.mockito:mockito-inline:4.7.0' + testImplementation 'org.mockito:mockito-junit-jupiter:4.8.0' + testImplementation 'org.mockito:mockito-core:4.8.0' + testImplementation 'org.mockito:mockito-inline:4.8.0' testImplementation 'org.junit.jupiter:junit-jupiter-api:5.9.0' testImplementation 'org.junit.jupiter:junit-jupiter-engine:5.9.0' testImplementation group: 'org.junit-pioneer', name: 'junit-pioneer', version: '1.7.1' diff --git a/src/main/java/com/firebolt/jdbc/connection/FireboltConnection.java b/src/main/java/com/firebolt/jdbc/connection/FireboltConnection.java index c6e479d20..fb0206ca9 100644 --- a/src/main/java/com/firebolt/jdbc/connection/FireboltConnection.java +++ b/src/main/java/com/firebolt/jdbc/connection/FireboltConnection.java @@ -24,6 +24,7 @@ import com.firebolt.jdbc.client.authentication.FireboltAuthenticationClient; import com.firebolt.jdbc.client.query.StatementClientImpl; import com.firebolt.jdbc.connection.settings.FireboltProperties; +import com.firebolt.jdbc.exception.ExceptionType; import com.firebolt.jdbc.exception.FireboltException; import com.firebolt.jdbc.exception.FireboltSQLFeatureNotSupportedException; import com.firebolt.jdbc.exception.FireboltUnsupportedOperationException; @@ -333,19 +334,25 @@ public boolean isValid(int timeout) throws SQLException { return false; } try { - validateConnection(this.getSessionProperties()); + validateConnection(this.getSessionProperties(), true); return true; } catch (Exception e) { return false; } } - private void validateConnection(FireboltProperties fireboltProperties) throws SQLException { + private void validateConnection(FireboltProperties fireboltProperties, boolean ignoreToManyRequestsError) throws SQLException { try (Statement s = createStatement(fireboltProperties)) { s.execute("SELECT 1"); - } catch (Exception e) { - log.warn("Connection is not valid", e); - throw e; + } catch (FireboltException e) { + //A connection is not invalid when too many requests are being sent. + //This error cannot be ignored when testing the connection to validate a param. + if (e.getType() == ExceptionType.TOO_MANY_REQUESTS && ignoreToManyRequestsError) { + log.warn("Too many requests are sent to the server", e); + } else { + log.warn("Connection is not valid", e); + throw e; + } } } @@ -365,7 +372,7 @@ public synchronized void addProperty(Pair property) throws Fireb try { FireboltProperties tmpProperties = FireboltProperties.copy(this.sessionProperties); tmpProperties.addProperty(property); - validateConnection(tmpProperties); + validateConnection(tmpProperties, false); this.sessionProperties.addProperty(property); } catch (FireboltException e) { throw e; diff --git a/src/main/java/com/firebolt/jdbc/exception/ExceptionType.java b/src/main/java/com/firebolt/jdbc/exception/ExceptionType.java index 02ee3e7ed..972be029e 100644 --- a/src/main/java/com/firebolt/jdbc/exception/ExceptionType.java +++ b/src/main/java/com/firebolt/jdbc/exception/ExceptionType.java @@ -6,5 +6,5 @@ */ public enum ExceptionType { ERROR, EXPIRED_TOKEN, TYPE_NOT_SUPPORTED, TYPE_TRANSFORMATION_ERROR, RESOURCE_NOT_FOUND, REQUEST_FAILED, - INVALID_REQUEST + INVALID_REQUEST, TOO_MANY_REQUESTS } \ No newline at end of file diff --git a/src/main/java/com/firebolt/jdbc/exception/FireboltException.java b/src/main/java/com/firebolt/jdbc/exception/FireboltException.java index 2058484cc..a0f4c859a 100644 --- a/src/main/java/com/firebolt/jdbc/exception/FireboltException.java +++ b/src/main/java/com/firebolt/jdbc/exception/FireboltException.java @@ -57,6 +57,8 @@ private ExceptionType getExceptionType(Integer httpStatusCode) { return INVALID_REQUEST; case SC_UNAUTHORIZED: return EXPIRED_TOKEN; + case SC_TOO_MANY_REQUESTS: + return TOO_MANY_REQUESTS; default: return ERROR; } diff --git a/src/test/java/com/firebolt/jdbc/connection/FireboltConnectionTest.java b/src/test/java/com/firebolt/jdbc/connection/FireboltConnectionTest.java index bf4fab05a..6bcf3382e 100644 --- a/src/test/java/com/firebolt/jdbc/connection/FireboltConnectionTest.java +++ b/src/test/java/com/firebolt/jdbc/connection/FireboltConnectionTest.java @@ -23,6 +23,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import com.firebolt.jdbc.connection.settings.FireboltProperties; +import com.firebolt.jdbc.exception.ExceptionType; import com.firebolt.jdbc.exception.FireboltException; import com.firebolt.jdbc.service.FireboltAuthenticationService; import com.firebolt.jdbc.service.FireboltEngineService; @@ -108,7 +109,7 @@ void shouldNotSetNewPropertyWhenConnectionIsNotValidWithTheNewProperty() throws fireboltAuthenticationService, fireboltEngineService, fireboltStatementService); when(fireboltStatementService.execute(any(), any(), any())) - .thenThrow(new IllegalArgumentException("The property is invalid")); + .thenThrow(new FireboltException(ExceptionType.TOO_MANY_REQUESTS)); assertThrows(FireboltException.class, () -> fireboltConnection.addProperty(new ImmutablePair<>("custom_1", "1"))); @@ -148,6 +149,24 @@ void shouldValidateConnectionWhenCallingIsValid() throws SQLException { assertEquals("SELECT 1", queryInfoWrapperArgumentCaptor.getValue().getSql()); } + @Test + void shouldIgnore429WhenValidatingConnection() throws SQLException { + when(fireboltStatementService.execute(any(), any(), any())) + .thenThrow(new FireboltException(ExceptionType.TOO_MANY_REQUESTS)); + FireboltConnection fireboltConnection = new FireboltConnection(URL, connectionProperties, + fireboltAuthenticationService, fireboltEngineService, fireboltStatementService); + assertTrue(fireboltConnection.isValid(500)); + } + + @Test + void shouldReturnFalseWhenValidatingConnectionThrowsAnException() throws SQLException { + when(fireboltStatementService.execute(any(), any(), any())) + .thenThrow(new FireboltException(ExceptionType.ERROR)); + FireboltConnection fireboltConnection = new FireboltConnection(URL, connectionProperties, + fireboltAuthenticationService, fireboltEngineService, fireboltStatementService); + assertFalse(fireboltConnection.isValid(500)); + } + @Test void shouldThrowExceptionWhenValidatingConnectionWithNegativeTimeout() throws SQLException { FireboltConnection fireboltConnection = new FireboltConnection(URL, connectionProperties,