Skip to content

Bump to Java 11 #150

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/master.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ jobs:
steps:
- uses: actions/checkout@v1
- uses: gradle/wrapper-validation-action@v1
- name: Set up JDK 1.8
- name: Set up JDK 11
uses: actions/setup-java@v1
with:
java-version: '8.0.282'
java-version: '11.0.23'
- name: build test and publish
run: ./gradlew assemble && ./gradlew check --info && ./gradlew publishToSonatype closeAndReleaseSonatypeStagingRepository -x check --info --stacktrace
4 changes: 2 additions & 2 deletions .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ jobs:
steps:
- uses: actions/checkout@v1
- uses: gradle/wrapper-validation-action@v1
- name: Set up JDK 1.8
- name: Set up JDK 11
uses: actions/setup-java@v1
with:
java-version: '8.0.282'
java-version: '11.0.23'
- name: build and test
run: ./gradlew assemble && ./gradlew check --info --stacktrace
4 changes: 2 additions & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ jobs:
steps:
- uses: actions/checkout@v1
- uses: gradle/wrapper-validation-action@v1
- name: Set up JDK 1.8
- name: Set up JDK 11
uses: actions/setup-java@v1
with:
java-version: '8.0.282'
java-version: '11.0.23'
- name: build test and publish
run: ./gradlew assemble && ./gradlew check --info && ./gradlew publishToSonatype closeAndReleaseSonatypeStagingRepository -x check --info --stacktrace
20 changes: 8 additions & 12 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,15 @@ plugins {
id 'java-library'
id 'maven-publish'
id 'signing'
id "biz.aQute.bnd.builder" version "6.2.0"
id "biz.aQute.bnd.builder" version "6.2.0"
id "io.github.gradle-nexus.publish-plugin" version "1.0.0"
}

java {
toolchain {
languageVersion = JavaLanguageVersion.of(11)
}
}

def getDevelopmentVersion() {
def output = new StringBuilder()
Expand All @@ -25,20 +30,11 @@ def getDevelopmentVersion() {
version
}

if (JavaVersion.current() != JavaVersion.VERSION_1_8) {
def msg = String.format("This build must be run with java 1.8 - you are running %s - gradle finds the JDK via JAVA_HOME=%s",
JavaVersion.current(), System.getenv("JAVA_HOME"))
throw new IllegalStateException(msg)
}


sourceCompatibility = 1.8
targetCompatibility = 1.8
def slf4jVersion = '1.7.30'
def releaseVersion = System.env.RELEASE_VERSION
version = releaseVersion ? releaseVersion : getDevelopmentVersion()
group = 'com.graphql-java'
description = 'A pure Java 8 port of Facebook Dataloader'
description = 'A pure Java 11 port of Facebook Dataloader'

gradle.buildFinished { buildResult ->
println "*******************************"
Expand Down Expand Up @@ -117,7 +113,7 @@ publishing {
asNode().children().last() + {
resolveStrategy = Closure.DELEGATE_FIRST
name 'java-dataloader'
description 'A pure Java 8 port of Facebook Dataloader'
description 'A pure Java 11 port of Facebook Dataloader'
url 'https://github.com/graphql-java/java-dataloader'
inceptionYear '2017'

Expand Down
20 changes: 10 additions & 10 deletions src/test/java/org/dataloader/DataLoaderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -692,13 +692,13 @@ public void should_work_with_duplicate_keys_when_caching_enabled() throws Execut
public void should_Accept_objects_with_a_complex_key() throws ExecutionException, InterruptedException {
List<Collection<JsonObject>> loadCalls = new ArrayList<>();
DataLoaderOptions options = newOptions().setCacheKeyFunction(getJsonObjectCacheMapFn());
DataLoader<JsonObject, Integer> identityLoader = idLoader(options, loadCalls);
DataLoader<JsonObject, JsonObject> identityLoader = idLoader(options, loadCalls);

JsonObject key1 = new JsonObject().put("id", 123);
JsonObject key2 = new JsonObject().put("id", 123);

CompletableFuture<Integer> future1 = identityLoader.load(key1);
CompletableFuture<Integer> future2 = identityLoader.load(key2);
CompletableFuture<JsonObject> future1 = identityLoader.load(key1);
CompletableFuture<JsonObject> future2 = identityLoader.load(key2);
identityLoader.dispatch();

await().until(() -> future1.isDone() && future2.isDone());
Expand All @@ -713,18 +713,18 @@ public void should_Accept_objects_with_a_complex_key() throws ExecutionException
public void should_Clear_objects_with_complex_key() throws ExecutionException, InterruptedException {
List<Collection<JsonObject>> loadCalls = new ArrayList<>();
DataLoaderOptions options = newOptions().setCacheKeyFunction(getJsonObjectCacheMapFn());
DataLoader<JsonObject, Integer> identityLoader = idLoader(options, loadCalls);
DataLoader<JsonObject, JsonObject> identityLoader = idLoader(options, loadCalls);

JsonObject key1 = new JsonObject().put("id", 123);
JsonObject key2 = new JsonObject().put("id", 123);

CompletableFuture<Integer> future1 = identityLoader.load(key1);
CompletableFuture<JsonObject> future1 = identityLoader.load(key1);
identityLoader.dispatch();

await().until(future1::isDone);
identityLoader.clear(key2); // clear equivalent object key

CompletableFuture<Integer> future2 = identityLoader.load(key1);
CompletableFuture<JsonObject> future2 = identityLoader.load(key1);
identityLoader.dispatch();

await().until(future2::isDone);
Expand All @@ -737,22 +737,22 @@ public void should_Clear_objects_with_complex_key() throws ExecutionException, I
public void should_Accept_objects_with_different_order_of_keys() throws ExecutionException, InterruptedException {
List<Collection<JsonObject>> loadCalls = new ArrayList<>();
DataLoaderOptions options = newOptions().setCacheKeyFunction(getJsonObjectCacheMapFn());
DataLoader<JsonObject, Integer> identityLoader = idLoader(options, loadCalls);
DataLoader<JsonObject, JsonObject> identityLoader = idLoader(options, loadCalls);

JsonObject key1 = new JsonObject().put("a", 123).put("b", 321);
JsonObject key2 = new JsonObject().put("b", 321).put("a", 123);

// Fetches as expected

CompletableFuture<Integer> future1 = identityLoader.load(key1);
CompletableFuture<Integer> future2 = identityLoader.load(key2);
CompletableFuture<JsonObject> future1 = identityLoader.load(key1);
CompletableFuture<JsonObject> future2 = identityLoader.load(key2);
identityLoader.dispatch();

await().until(() -> future1.isDone() && future2.isDone());
assertThat(loadCalls, equalTo(singletonList(singletonList(key1))));
assertThat(loadCalls.size(), equalTo(1));
assertThat(future1.get(), equalTo(key1));
assertThat(future2.get(), equalTo(key1));
assertThat(future2.get(), equalTo(key2));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry - why is this test changed for Java 11 ? was it wrong? and if so - why was it passing ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A brief inspection indicates the test shouldn't have been working with Java 8. We can see this because the identityLoader doesn't have identical types for its K and V type parameters - if we pass in a JsonObject to a loader generated from idLoader, we should expect to receive that same JsonObject coming out, but that's not what this test was checking.

As to why it was passing at all - I suspect it has something to do with this bug that was fixed with Java 9. Without digging into the javac-generated bytecode, it looks like a class check was missing from bytecode generated with Java 8. As such, when we tried to run with this code with Java 11, we started failing because we tried to cast a JsonObject (the returned value) to an Integer (the V in the DataLoader).

As to why I changed it from key1 to key2 - while key1.equals(key2) is true, it seemed neater to check we got key2 for the future (as it corresponded to future2).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I just looked into it - <K,V> are all just Object at runtime and since we never treated a value as a JsonObject - well the test got away with it. Makes sense to fix

}

@Test
Expand Down
Loading