Skip to content
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

Contextualise CF tck #197

Merged
merged 2 commits into from
Sep 2, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import org.eclipse.microprofile.context.tck.contexts.priority.spi.ThreadPriorityContextProvider;
import org.eclipse.microprofile.context.ManagedExecutor;
import org.eclipse.microprofile.context.ThreadContext;
import org.eclipse.microprofile.context.spi.ContextManager;
import org.eclipse.microprofile.context.spi.ThreadContextProvider;
import org.jboss.arquillian.container.test.api.Deployment;
import org.jboss.arquillian.testng.Arquillian;
Expand Down Expand Up @@ -123,7 +124,7 @@ public static WebArchive createDeployment() {
BufferContextProvider.class, LabelContextProvider.class, ThreadPriorityContextProvider.class);

return ShrinkWrap.create(WebArchive.class, ManagedExecutorTest.class.getSimpleName() + ".war")
.addClass(ManagedExecutorTest.class)
.addClasses(ManagedExecutorTest.class, TckThread.class, TckThreadFactory.class, ThreadContextTest.class)
.addAsLibraries(fakeContextProviders);
}

Expand Down Expand Up @@ -2759,4 +2760,202 @@ public void untimedInvokeAnyRunsTasksWithContext()
Label.set(null);
}
}

/**
* Verify that we can copy a CompletableFuture and get context propagation in the copy's dependent stages.
*
* @throws InterruptedException indicates test failure
* @throws ExecutionException indicates test failure
* @throws TimeoutException indicates test failure
*/
@Test
public void copyCompletableFuture() throws InterruptedException, ExecutionException, TimeoutException {
ManagedExecutor executor = ManagedExecutor.builder()
.maxAsync(1)
.propagated(Label.CONTEXT_NAME)
.cleared(ThreadContext.ALL_REMAINING)
.build();

try {
// Set non-default values
Label.set("copy-test-label-A");

CompletableFuture<String> noContextCF = new CompletableFuture<>();
CompletableFuture<String> noContextCFStage1 = noContextCF.thenApplyAsync(res -> {
Assert.assertEquals(Label.get(), "",
"Context type should not be propagated to non-contextual action.");

return res;
});

CompletableFuture<String> contextCF = executor.copy(noContextCFStage1).thenApplyAsync(res -> {
Assert.assertEquals(Label.get(), "copy-test-label-A",
"Context type should be propagated to contextual action.");

return res;
});

noContextCF.complete("OK");
contextCF.get(MAX_WAIT_NS, TimeUnit.NANOSECONDS);
}
finally {
executor.shutdownNow();
// Restore original values
Label.set(null);
}
}

/**
* Verify that we can copy a CompletionStage and get context propagation in the copy's dependent stages.
*
* @throws InterruptedException indicates test failure
* @throws ExecutionException indicates test failure
* @throws TimeoutException indicates test failure
*/
@Test
public void copyCompletionStage() throws InterruptedException, ExecutionException, TimeoutException {
ManagedExecutor executor = ManagedExecutor.builder()
.maxAsync(1)
.propagated(Label.CONTEXT_NAME)
.cleared(ThreadContext.ALL_REMAINING)
.build();

try {
// Set non-default values
Label.set("copy-test-label-A");

CompletableFuture<String> noContextCF = new CompletableFuture<>();
CompletionStage<String> noContextCFStage1 = noContextCF.thenApplyAsync(res -> {
Assert.assertEquals(Label.get(), "",
"Context type should not be propagated to non-contextual action.");

return res;
});

CompletionStage<String> contextCF = executor.copy(noContextCFStage1).thenApplyAsync(res -> {
Assert.assertEquals(Label.get(), "copy-test-label-A",
"Context type should be propagated to contextual action.");

return res;
});

noContextCF.complete("OK");
contextCF.toCompletableFuture().get(MAX_WAIT_NS, TimeUnit.NANOSECONDS);
}
finally {
executor.shutdownNow();
// Restore original values
Label.set(null);
}
}

/**
* Verify that we can obtain a ThreadContext with the same settings as the ManagedExecutor
*
* @throws InterruptedException indicates test failure
* @throws ExecutionException indicates test failure
* @throws TimeoutException indicates test failure
*/
@Test
public void threadContextHasSamePropagationSettings() throws InterruptedException, ExecutionException, TimeoutException {
ManagedExecutor executor = ManagedExecutor.builder()
.maxAsync(1)
.propagated(Label.CONTEXT_NAME)
.cleared(ThreadContext.ALL_REMAINING)
.build();

try {
// Set non-default values
Label.set("thread-context-test-label-A");
Buffer.set(new StringBuffer("thread-context-test-buffer-A"));

executor.getThreadContext().contextualRunnable(() -> {
Assert.assertEquals(Label.get(), "thread-context-test-label-A",
"getThreadContext call is lacking propagation of Label context.");

Assert.assertEquals(Buffer.get().toString(), "",
"getThreadContext call is lacking clearance of Buffer context.");
}).run();
}
finally {
executor.shutdownNow();
// Restore original values
Label.set(null);
Buffer.set(null);
}
}

/**
* Verify that the presence of a default executor service implies async actions run on it.
*
* @throws ExecutionException indicates test failure
* @throws InterruptedException indicates test failure
* @throws TimeoutException indicates test failure
*/
@Test
public void withDefaultExecutorServiceIsUsedDirectlyAndViaGetThreadContext()
throws ExecutionException, InterruptedException, TimeoutException {
ExecutorService executorService = Executors.newSingleThreadExecutor(new TckThreadFactory());
ContextManager contextManager = ThreadContextTest.getContextManagerBuilderIfSupported()
Copy link
Contributor

Choose a reason for hiding this comment

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

This fails with

java.lang.NoClassDefFoundError: org.eclipse.microprofile.context.tck.ThreadContextTest

To fix, just need to add ThreadContextTest.class under addClasses

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooops, done.

.withDefaultExecutorService(executorService)
.addDiscoveredThreadContextProviders()
.build();
Copy link
Contributor

Choose a reason for hiding this comment

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

ContextManagerProvider.getContextManagerBuilder() is optional. We need to allow for implementations that don't support it. I believe this can be done as follows:

        ContextManager.Builder contextManagerBuilder;
        try {
            contextManagerBuilder = ContextManagerProvider.instance().getContextManagerBuilder();
        } catch (UnsupportedOperationException x) {
            // getContextManagerBuilder is optional. Skip if unimplemented.
            return;
        }
        ContextManager contextManager = contextManagerBuilder
                .addDiscoveredThreadContextProviders()
                .withDefaultExecutorService(null)
                .build();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ouch, I didn't realise that. But then how can I test the executor service in this case? I need to make sure that it works if it's specified and if it's set to null. If I use the default context manager I can't know.

Copy link
Contributor

Choose a reason for hiding this comment

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

If an implementation doesn't support getContextManagerBuilder, that means there is no way to configure the default executor service. The implementation will either have a built-in default that it uses for async dependent stages of withContextCapture, or it won't have a built-in default, which will mean UnsupportedOperationException is appropriate when attempting to create async stages. In order to test this, we can trap for UnsupportedOperationException on thenApplyAsync(fn), which is considered a passing result when getContextManagerBuilder is rejected, and in the absence of the exception, check that async dependent stages that are created from the copied stage run successfully, independent of any constraints that might be in place on the executor of the stage from which it was originally copied.

Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify, your test is fine, except that it needs to trap UnsupportedOperationException and skip itself in that case. And then a separate test can be written to cover the default context manager, in which the test will need to trap UnsupportedOperationException on thenApplyAsync (or whatever *Async method is used)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, fine, is this better then?

Copy link
Contributor

Choose a reason for hiding this comment

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

Mostly, but two other issues were introduced in the changes. I added comments for those inline instead of under this conversation.

ManagedExecutor executor = contextManager.newManagedExecutorBuilder()
.propagated(Label.CONTEXT_NAME)
.cleared(ThreadContext.ALL_REMAINING)
.build();
try {
// Set non-default values
Label.set("default-executor-service-managed-executor-test-label-A");


// try it directly
CompletableFuture<String> contextCF = executor.newIncompleteFuture();
CompletableFuture<String> contextCFStage1 = contextCF.thenApplyAsync(res -> {
Assert.assertEquals(Label.get(), "default-executor-service-managed-executor-test-label-A",
"Context type should be propagated to contextual CompletableFuture.");

Assert.assertTrue(Thread.currentThread() instanceof TckThread,
"Current thread should have been created by default executor service");

return res;
});

// and indirectly
CompletableFuture<String> noContextCF = new CompletableFuture<>();
CompletableFuture<String> contextCFStage2 = executor.getThreadContext().withContextCapture(noContextCF);
CompletableFuture<String> contextCFStage3 = contextCFStage2.thenApplyAsync(res -> {
Assert.assertEquals(Label.get(), "default-executor-service-managed-executor-test-label-A",
"Context type should be propagated to contextual CompletableFuture.");

Assert.assertTrue(Thread.currentThread() instanceof TckThread,
"Current thread should have been created by default executor service");

return res;
});

CompletionStage<String> contextCSStage1 = executor.getThreadContext().withContextCapture((CompletionStage<String>)noContextCF);
CompletionStage<String> contextCSStage2 = contextCSStage1.thenApplyAsync(res -> {
Assert.assertEquals(Label.get(), "default-executor-service-managed-executor-test-label-A",
"Context type should be propagated to contextual CompletionStage.");

Assert.assertTrue(Thread.currentThread() instanceof TckThread,
"Current thread should have been created by default executor service");

return res;
});

noContextCF.complete("OK");
contextCF.complete("OK");
contextCFStage1.get(MAX_WAIT_NS, TimeUnit.NANOSECONDS);
contextCFStage3.get(MAX_WAIT_NS, TimeUnit.NANOSECONDS);
contextCSStage2.toCompletableFuture().get(MAX_WAIT_NS, TimeUnit.NANOSECONDS);
}
finally {
// Restore original values
Label.set(null);
executorService.shutdownNow();
executor.shutdownNow();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright (c) 2018,2019 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional
* information regarding copyright ownership.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* You may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.eclipse.microprofile.context.tck;

public class TckThread extends Thread {

public TckThread(Runnable r) {
super(r);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright (c) 2018,2019 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional
* information regarding copyright ownership.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* You may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.eclipse.microprofile.context.tck;

import java.util.concurrent.ThreadFactory;

public class TckThreadFactory implements ThreadFactory {

@Override
public Thread newThread(Runnable r) {
return new TckThread(r);
}

}
Loading