Add tests for ManagedThreadFactory Injected to Servlet

[aubi]

No description provided.

[njr-11]

The asserted value and message are contradictory. The value when looked up is 0 (default).

Suggested change

	assertEquals(results.poll(MAX_WAIT_SECONDS, TimeUnit.SECONDS), 161,
	"Third-party context type IntContext must be propagated to thread "
	+ "per ManagedThreadFactoryDefinition and ContextServiceDefinition configuration "
	+ "based on the thread context at the time the ManagedThreadFactory was looked up.");
	assertEquals(results.poll(MAX_WAIT_SECONDS, TimeUnit.SECONDS), 0,
	"Third-party context type IntContext must be propagated to thread "
	+ "per ManagedThreadFactoryDefinition and ContextServiceDefinition configuration "
	+ "based on the thread context at the time the ManagedThreadFactory was looked up.");

[njr-11]

IntContext is not used in this test. It should not be necessary to reset it.

Suggested change

IntContext.set(0);

[njr-11]

This has a copy/paste error from the other test that had IntContext=1000 on the thread when the lookup was performed. For this one, the IntContext is 0 when looked up, and needs to be updated as follows:

Suggested change

	IntContext.set(2000);
	StringContext.set("testParallelStreamBackedByManagedThreadFactory-2");
	fj = new ForkJoinPool(4, threadFactoryInjA, null, false);
	IntContext.set(3000);
	StringContext.set("testParallelStreamBackedByManagedThreadFactory-3");
	ForkJoinTask<Optional<Integer>> task = fj.submit(() -> {
	return Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9).parallelStream().map(num -> {
	assertEquals(StringContext.get(), "",
	"Third-party context type StringContext must be cleared on " + "ForkJoin thread.");
	try {
	assertNotNull(InitialContext.doLookup("java:app/concurrent/ContextA"),
	"Application context must be propagated to ForkJoin thread");
	} catch (NamingException x) {
	throw new CompletionException(x);
	}
	return num * Thread.currentThread().getPriority() + IntContext.get();
	}).reduce(Integer::sum);
	});
	Optional<Integer> result = task.join();
	assertEquals(result.get(), Integer.valueOf(9180),
	"Third-party context type IntContext must propagated to ForkJoin threads "
	+ "(thousands digit should be 9) and thread priority (4) must be enforced "
	+ "on ForkJoin threads (hundreds/tens/ones digits must be 4x5x9=180) "
	+ "per configuration of the ManagedThreadFactoryDefinition and ContextServiceDefinition.");
	IntContext.set(2000);
	StringContext.set("testParallelStreamBackedByManagedThreadFactory-2");
	fj = new ForkJoinPool(4, threadFactoryInjA, null, false);
	IntContext.set(3000);
	StringContext.set("testParallelStreamBackedByManagedThreadFactory-3");
	ForkJoinTask<Optional<Integer>> task = fj.submit(() -> {
	return Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9).parallelStream().map(num -> {
	assertEquals(StringContext.get(), "",
	"Third-party context type StringContext must be cleared on " + "ForkJoin thread.");
	try {
	assertNotNull(InitialContext.doLookup("java:app/concurrent/ContextA"),
	"Application context must be propagated to ForkJoin thread");
	} catch (NamingException x) {
	throw new CompletionException(x);
	}
	return num * Thread.currentThread().getPriority() + IntContext.get();
	}).reduce(Integer::sum);
	});
	Optional<Integer> result = task.join();
	assertEquals(result.get(), Integer.valueOf(180),
	"Third-party context type IntContext must be propagated to ForkJoin threads "
	+ "(thousands digit should be 0) and thread priority (4) must be enforced "
	+ "on ForkJoin threads (hundreds/tens/ones digits must be 4x5x9=180) "
	+ "per configuration of the ManagedThreadFactoryDefinition and ContextServiceDefinition.");

[njr-11]

IntContext is 0 (default) when looked up.

Suggested change

	assertEquals(results.poll(MAX_WAIT_SECONDS, TimeUnit.SECONDS), Integer.valueOf(161),
	"Third-party context type IntContext must be propagated to thread "
	+ "per ManagedThreadFactoryDefinition and ContextServiceDefinition configuration "
	+ "based on the thread context at the time the ManagedThreadFactory was looked up.");
	assertEquals(results.poll(MAX_WAIT_SECONDS, TimeUnit.SECONDS), Integer.valueOf(0),
	"Third-party context type IntContext must be propagated to thread "
	+ "per ManagedThreadFactoryDefinition and ContextServiceDefinition configuration "
	+ "based on the thread context at the time the ManagedThreadFactory was looked up.");

[njr-11]

The Runnable named "lookupTask" wasn't actually performing a lookup, just returning an existing value, which didn't test context propagation at all.

Suggested change

	lookupTaskResult.complete(threadFactoryB);
	lookupTaskResult.complete(InitialContext.doLookup("java:comp/concurrent/EJBThreadFactoryB"));

[njr-11]

You can remove this if you want to. The test doesn't use IntContext so it shouldn't need to be cleared.

Suggested change

	IntContext.set(0);
	IntContext.set(0);

[njr-11]

Same as other comment - this looks like a copy/paste error from a test where IntContext=1000 was on the thread during lookup. In this case, IntContext is 0 (defaulted) during lookup.

Suggested change

	IntContext.set(2000);
	StringContext.set("testParallelStreamBackedByManagedThreadFactoryEJB-2");
	fj = new ForkJoinPool(4, threadFactoryA, null, false);
	IntContext.set(3000);
	StringContext.set("testParallelStreamBackedByManagedThreadFactoryEJB-3");
	ForkJoinTask<Optional<Integer>> task = fj.submit(() -> {
	return Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9).parallelStream().map(num -> {
	assertEquals(StringContext.get(), "",
	"Third-party context type StringContext must be cleared on " + "ForkJoin thread.");
	try {
	assertNotNull(InitialContext.doLookup("java:app/concurrent/ContextA"),
	"Application context must be propagated to ForkJoin thread");
	} catch (NamingException x) {
	throw new CompletionException(x);
	}
	return num * Thread.currentThread().getPriority() + IntContext.get();
	}).reduce(Integer::sum);
	});
	Optional<Integer> result = task.join();
	assertEquals(result.get(), Integer.valueOf(9180),
	"Third-party context type IntContext must propagated to ForkJoin threads "
	+ "(thousands digit should be 9) and thread priority (4) must be enforced "
	+ "on ForkJoin threads (hundreds/tens/ones digits must be 4x5x9=180) "
	+ "per configuration of the ManagedThreadFactoryDefinition and ContextServiceDefinition.");
	IntContext.set(2000);
	StringContext.set("testParallelStreamBackedByManagedThreadFactoryEJB-2");
	fj = new ForkJoinPool(4, threadFactoryA, null, false);
	IntContext.set(3000);
	StringContext.set("testParallelStreamBackedByManagedThreadFactoryEJB-3");
	ForkJoinTask<Optional<Integer>> task = fj.submit(() -> {
	return Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9).parallelStream().map(num -> {
	assertEquals(StringContext.get(), "",
	"Third-party context type StringContext must be cleared on " + "ForkJoin thread.");
	try {
	assertNotNull(InitialContext.doLookup("java:app/concurrent/ContextA"),
	"Application context must be propagated to ForkJoin thread");
	} catch (NamingException x) {
	throw new CompletionException(x);
	}
	return num * Thread.currentThread().getPriority() + IntContext.get();
	}).reduce(Integer::sum);
	});
	Optional<Integer> result = task.join();
	assertEquals(result.get(), Integer.valueOf(180),
	"Third-party context type IntContext must be propagated to ForkJoin threads "
	+ "(thousands digit should be 0) and thread priority (4) must be enforced "
	+ "on ForkJoin threads (hundreds/tens/ones digits must be 4x5x9=180) "
	+ "per configuration of the ManagedThreadFactoryDefinition and ContextServiceDefinition.");

[OndroMih]

As I commented on #212, I cannot support it. I believe behavior has never been clearly specified in the spec. GlassFish, which used to be the reference impl doesn’t comply with it.

I’d like to see opinions from other implementors and users of other implementations before I and the GlassFish team can support this.

[aubi]

@njr-11
Thanks a lot for your comments! I'll fix the issues.

@OndroMih

1. I think that injection is the preferred way of using MES etc. So having some tests makes sense.
2. Regarding the issue with storing context -- this test shows, what exactly I meant and I would like to clarify, how it should behave. But I'll fix the Nathan's issues first.