From dfcba9c699f7403eff840bb6e0eeec4b4586f755 Mon Sep 17 00:00:00 2001 From: Lukasz Klich Date: Wed, 17 Nov 2021 19:25:12 +0100 Subject: [PATCH] SDK-1546 evict transactional HTTP client connection pool (#224) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SDK-1546 evict transactional HTTP client connection pool When receiving PNStatusCategory.PNReconnectedCategory users can think that is about the whole PubNub library and not only about the subscription loop. For better user experience we're going to evict possibly broken connections for transactional calls Co-authored-by: Bartek Kępa --- .github/CODEOWNERS | 4 +- .github/workflows/run_acceptance_tests.yml | 20 +++++-- .pubnub.yml | 17 +++--- CHANGELOG.md | 6 +++ README.md | 2 +- build.gradle | 54 +++++-------------- src/main/java/com/pubnub/api/PubNub.java | 4 +- .../pubnub/api/managers/RetrofitManager.java | 24 ++++++++- src/test/java/com/pubnub/api/PubNubTest.java | 2 +- .../pubnub/contract/RunMainCucumberTest.kt | 19 +++++++ 10 files changed, 92 insertions(+), 60 deletions(-) create mode 100644 src/test/java/com/pubnub/contract/RunMainCucumberTest.kt diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 878ceb3a2..ad9be5fbe 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,3 +1,3 @@ -* @kleewho @bartk @Chesteer89 -.github/* @parfeon @kleewho @bartk @Chesteer89 +* @kleewho @bartk @Chesteer89 @budgetpreneur +.github/* @parfeon @kleewho @bartk @Chesteer89 @budgetpreneur README.md @techwritermat @kazydek \ No newline at end of file diff --git a/.github/workflows/run_acceptance_tests.yml b/.github/workflows/run_acceptance_tests.yml index 15840c707..f671f2986 100644 --- a/.github/workflows/run_acceptance_tests.yml +++ b/.github/workflows/run_acceptance_tests.yml @@ -20,21 +20,31 @@ jobs: uses: ./client-engineering-deployment-tools/actions/mock-server with: token: ${{ secrets.GH_TOKEN }} - - name: Run acceptance tests + - name: Run acceptance tests (optional) run: | export pubKey=somePubKey export subKey=someSubKey export pamPubKey=somePamPubKey export pamSubKey=somePamSubKey export pamSecKey=someSecKey - export featuresDir=sdk-specifications/features/access - export cucumberTags=@feature=access export serverHostPort=localhost:8090 export serverMock=true - ./gradlew cucumber + ./gradlew cucumber --tests RunBetaCucumberTest -Dcucumber.features="sdk-specifications/features/" -i + continue-on-error: true + - name: Run acceptance tests (required) + run: | + export pubKey=somePubKey + export subKey=someSubKey + export pamPubKey=somePamPubKey + export pamSubKey=somePamSubKey + export pamSecKey=someSecKey + export serverHostPort=localhost:8090 + export serverMock=true + ./gradlew cucumber --tests RunMainCucumberTest -Dcucumber.features="sdk-specifications/features/" -i - name: Expose acceptance tests reports uses: actions/upload-artifact@v2 + if: always() with: name: acceptance-test-reports path: ./build/reports/cucumber-reports - + retention-days: 7 diff --git a/.pubnub.yml b/.pubnub.yml index b6c6f5601..3b8c360fb 100644 --- a/.pubnub.yml +++ b/.pubnub.yml @@ -1,9 +1,9 @@ name: java -version: v5.2.2 +version: v5.2.3 schema: 1 scm: github.com/pubnub/java files: - - build/libs/pubnub-gson-5.2.2-all.jar + - build/libs/pubnub-gson-5.2.3-all.jar sdks: - type: library @@ -23,8 +23,8 @@ sdks: - distribution-type: library distribution-repository: git release - package-name: pubnub-gson-5.2.2 - location: https://github.com/pubnub/java/releases/download/v5.2.2/pubnub-gson-5.2.2-all.jar + package-name: pubnub-gson-5.2.3 + location: https://github.com/pubnub/java/releases/download/v5.2.3/pubnub-gson-5.2.3-all.jar supported-platforms: supported-operating-systems: Android: @@ -135,8 +135,8 @@ sdks: - distribution-type: library distribution-repository: maven - package-name: pubnub-gson-5.2.2 - location: https://repo.maven.apache.org/maven2/com/pubnub/pubnub-gson/5.2.2/pubnub-gson-5.2.2.jar + package-name: pubnub-gson-5.2.3 + location: https://repo.maven.apache.org/maven2/com/pubnub/pubnub-gson/5.2.3/pubnub-gson-5.2.3.jar supported-platforms: supported-operating-systems: Android: @@ -234,6 +234,11 @@ sdks: is-required: Required changelog: + - date: 2021-11-17 + version: v5.2.3 + changes: + - type: bug + text: "Eviction of OkHttp connection pool after reestablishing connection (affects transactional calls)." - date: 2021-11-04 version: v5.2.2 changes: diff --git a/CHANGELOG.md b/CHANGELOG.md index 553914ce4..b36492216 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## v5.2.3 +November 17 2021 + +#### Fixed +- Eviction of OkHttp connection pool after reestablishing connection (affects transactional calls). + ## v5.2.2 November 04 2021 diff --git a/README.md b/README.md index d4b8e2819..ab2e5e6b9 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ You will need the publish and subscribe keys to authenticate your app. Get your com.pubnub pubnub-gson - 5.2.2 + 5.2.3 ``` diff --git a/build.gradle b/build.gradle index bfbc6fe4d..f69148076 100644 --- a/build.gradle +++ b/build.gradle @@ -12,7 +12,7 @@ plugins { } group = 'com.pubnub' -version = '5.2.2' +version = '5.2.3' description = """""" @@ -23,47 +23,6 @@ targetCompatibility = 1.8 configurations.all { } -configurations { - cucumberRuntime { - extendsFrom testImplementation - } -} - -task cucumber() { - dependsOn assemble, testClasses - group = "verification" - doLast { - javaexec { - Properties localProperties = new Properties() - try { - localProperties.load(new FileInputStream(rootProject.file("test.properties"))) - } - catch (Exception e) { - localProperties.putAll(System.getenv()) - } - main = "io.cucumber.core.cli.Main" - classpath = configurations.cucumberRuntime + sourceSets.main.output + sourceSets.test.output - - if (localProperties['cucumber.tags'] != null) { - args = ['--tags', localProperties['cucumberTags'], - '--plugin', 'pretty', - '--plugin', 'json:build/reports/cucumber-reports/Cucumber.json', - '--plugin', 'junit:build/reports/cucumber-reports/Cucumber.xml', - '--plugin', 'html:build/reports/cucumber-reports/Cucumber.html', - '--glue', 'com.pubnub.contract', - localProperties['featuresDir']] - } else { - args = ['--plugin', 'pretty', - '--plugin', 'json:build/reports/cucumber-reports/Cucumber.json', - '--plugin', 'junit:build/reports/cucumber-reports/Cucumber.xml', - '--plugin', 'html:build/reports/cucumber-reports/Cucumber.html', - '--glue', 'com.pubnub.contract', - localProperties['featuresDir']] - } - } - } -} - lombok { version = "1.18.4" } @@ -240,4 +199,15 @@ task sourcesJar(type: Jar, dependsOn: classes) { from "$buildDir/delombok" } +task cucumber(type: Test) { + systemProperty "cucumber.filter.tags", System.getProperty("cucumber.filter.tags") + systemProperty "cucumber.features", System.getProperty("cucumber.features") + systemProperty "cucumber.plugins", System.getProperty("cucumber.plugins") +} + +test { + exclude '**/contract/*.class' +} + + build.finalizedBy(shadowJar) diff --git a/src/main/java/com/pubnub/api/PubNub.java b/src/main/java/com/pubnub/api/PubNub.java index ac9df69d4..af492497c 100644 --- a/src/main/java/com/pubnub/api/PubNub.java +++ b/src/main/java/com/pubnub/api/PubNub.java @@ -103,7 +103,7 @@ public class PubNub { private static final int TIMESTAMP_DIVIDER = 1000; private static final int MAX_SEQUENCE = 65535; - private static final String SDK_VERSION = "5.2.2"; + private static final String SDK_VERSION = "5.2.3"; private final ListenerManager listenerManager; private final StateManager stateManager; @@ -114,8 +114,8 @@ public PubNub(@NotNull PNConfiguration initialConfig) { this.mapper = new MapperManager(); this.telemetryManager = new TelemetryManager(); this.basePathManager = new BasePathManager(initialConfig); - this.retrofitManager = new RetrofitManager(this); this.listenerManager = new ListenerManager(this); + this.retrofitManager = new RetrofitManager(this); this.stateManager = new StateManager(this.configuration); this.tokenManager = new TokenManager(); final ReconnectionManager reconnectionManager = new ReconnectionManager(this); diff --git a/src/main/java/com/pubnub/api/managers/RetrofitManager.java b/src/main/java/com/pubnub/api/managers/RetrofitManager.java index 32b8678ed..282f232b0 100644 --- a/src/main/java/com/pubnub/api/managers/RetrofitManager.java +++ b/src/main/java/com/pubnub/api/managers/RetrofitManager.java @@ -1,11 +1,13 @@ package com.pubnub.api.managers; - import com.pubnub.api.PNConfiguration; import com.pubnub.api.PubNub; +import com.pubnub.api.callbacks.SubscribeCallback; import com.pubnub.api.endpoints.vendor.AppEngineFactory; import com.pubnub.api.enums.PNLogVerbosity; +import com.pubnub.api.enums.PNStatusCategory; import com.pubnub.api.interceptors.SignatureInterceptor; +import com.pubnub.api.models.consumer.PNStatus; import com.pubnub.api.services.AccessManagerService; import com.pubnub.api.services.ChannelGroupService; import com.pubnub.api.services.ChannelMetadataService; @@ -23,10 +25,12 @@ import lombok.Getter; import okhttp3.OkHttpClient; import okhttp3.logging.HttpLoggingInterceptor; +import org.jetbrains.annotations.NotNull; import retrofit2.Retrofit; import java.util.Collections; import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; public class RetrofitManager { @@ -104,6 +108,24 @@ public RetrofitManager(PubNub pubNubInstance) { this.pubnub.getConfiguration().getConnectTimeout() ).retryOnConnectionFailure(false) ); + + //Because our users can think that PNStatusCategory.PNReconnectedCategory is about the whole + //PubNub library and not only about the subscription loop just for safety we're going to + //evict possibly broken connections for transactional calls + this.pubnub.addListener(new SubscribeCallback.BaseSubscribeCallback() { + @Override + public void status(@NotNull final PubNub pubnub, @NotNull final PNStatus pnStatus) { + if (pnStatus.getCategory() == PNStatusCategory.PNReconnectedCategory) { + //On Android this callback is run on main thread therefore this thread is necessary + Executors.newSingleThreadExecutor().execute(new Runnable() { + @Override + public void run() { + transactionClientInstance.connectionPool().evictAll(); + } + }); + } + } + }); } this.transactionInstance = createRetrofit(this.transactionClientInstance); diff --git a/src/test/java/com/pubnub/api/PubNubTest.java b/src/test/java/com/pubnub/api/PubNubTest.java index a56d51710..539af9092 100644 --- a/src/test/java/com/pubnub/api/PubNubTest.java +++ b/src/test/java/com/pubnub/api/PubNubTest.java @@ -99,7 +99,7 @@ public void getVersionAndTimeStamp() { pubnub = new PubNub(pnConfiguration); String version = pubnub.getVersion(); int timeStamp = pubnub.getTimestamp(); - Assert.assertEquals("5.2.2", version); + Assert.assertEquals("5.2.3", version); Assert.assertTrue(timeStamp > 0); } diff --git a/src/test/java/com/pubnub/contract/RunMainCucumberTest.kt b/src/test/java/com/pubnub/contract/RunMainCucumberTest.kt new file mode 100644 index 000000000..26ee75260 --- /dev/null +++ b/src/test/java/com/pubnub/contract/RunMainCucumberTest.kt @@ -0,0 +1,19 @@ +package com.pubnub.contract + +import io.cucumber.junit.Cucumber +import io.cucumber.junit.CucumberOptions +import org.junit.runner.RunWith + +@RunWith(Cucumber::class) +@CucumberOptions( + tags = "not @skip and not @na=ruby and not @beta", + plugin = ["pretty", "summary", "junit:build/reports/cucumber-reports/main.xml"] +) +class RunMainCucumberTest + +@RunWith(Cucumber::class) +@CucumberOptions( + tags = "not @skip and not @na=ruby and @beta", + plugin = ["pretty", "summary", "junit:build/reports/cucumber-reports/beta.xml"] +) +class RunBetaCucumberTest