Skip to content

Commit

Permalink
SDK-1546 evict transactional HTTP client connection pool (#224)
Browse files Browse the repository at this point in the history
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 <bartlomiej.kepa@pubnub.com>
  • Loading branch information
kleewho and bartk authored Nov 17, 2021
1 parent 1790390 commit dfcba9c
Show file tree
Hide file tree
Showing 10 changed files with 92 additions and 60 deletions.
4 changes: 2 additions & 2 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -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
20 changes: 15 additions & 5 deletions .github/workflows/run_acceptance_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
17 changes: 11 additions & 6 deletions .pubnub.yml
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ You will need the publish and subscribe keys to authenticate your app. Get your
<dependency>
<groupId>com.pubnub</groupId>
<artifactId>pubnub-gson</artifactId>
<version>5.2.2</version>
<version>5.2.3</version>
</dependency>
```

Expand Down
54 changes: 12 additions & 42 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ plugins {
}
group = 'com.pubnub'

version = '5.2.2'
version = '5.2.3'

description = """"""

Expand All @@ -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"
}
Expand Down Expand Up @@ -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)
4 changes: 2 additions & 2 deletions src/main/java/com/pubnub/api/PubNub.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand Down
24 changes: 23 additions & 1 deletion src/main/java/com/pubnub/api/managers/RetrofitManager.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/com/pubnub/api/PubNubTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
19 changes: 19 additions & 0 deletions src/test/java/com/pubnub/contract/RunMainCucumberTest.kt
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit dfcba9c

Please sign in to comment.