SimpleNamingContextBuilder: Second builder does not change JNDI context after first has been deactivated [SPR-17505]

[spring-issuemaster]

Eelco de Lang opened SPR-17505 and commented

When creating the SimpleNamingContextBuilder for a second time, it doesn't work well. Different testcases can cause eachother to fail. The second will be missing it's JDNI settings.
This is caused by the fact that the first SimpleNamingContextBuilder is still connected to the NamingManager.

@Test
public void testSimpleNamingContextBuilder()
	throws NamingException, MalformedURLException, IllegalStateException
{
	SimpleNamingContextBuilder builder = SimpleNamingContextBuilder.emptyActivatedContextBuilder();
	builder.bind("url/testname", "WillNotBeCleared");
	builder.deactivate();

	SimpleNamingContextBuilder builder2 = SimpleNamingContextBuilder.emptyActivatedContextBuilder();
	builder2.clear();
	builder2.bind("url/testname", "ThisIsNotSet");

	InitialContext initialContext2 = new InitialContext();
	assertEquals("WillNotBeCleared", initialContext2.lookup("url/testname"));
}

---

Affects: 4.3.14, 5.1.2

[spring-issuemaster]

Eelco de Lang commented

As a workaround I use:

@SuppressWarnings("PMD.UseUtilityClass")
public class ReusableNamingContextBuilder
	extends SimpleNamingContextBuilder
{
	private static ReusableNamingContextBuilder builder;

	protected ReusableNamingContextBuilder()
	{
		super();
	}

	public static ReusableNamingContextBuilder emptyActivatedContextBuilder()
		throws NamingException
	{
		synchronized (ReusableNamingContextBuilder.class) {
			if (builder == null) {
				builder = new ReusableNamingContextBuilder();
			}
		}
		builder.clear();
		builder.activate();
		return builder;
	}
}

[spring-issuemaster]

Juergen Hoeller commented

In your test code, why do you call builder.deactivate() and builder.clear() explicitly? Keeping the original builder activated makes the test pass, and clear() is a no-op after an emptyActivatedContextBuilder call...

And through your workaround, you effectively make deactivate() a no-op since you hang on to same builder instance nevertheless, taking it from your own field rather than from activated. We could only really fix this through skipping deactivation as well which seems backwards given that you're calling deactivate() explicitly.

[spring-issuemaster]

Eelco de Lang commented

Maybe I oversimplified my problem a bit. This maybe this looks a bit more realistic:

	@BeforeClass
	public static void testSuite()
		throws NamingException, MalformedURLException, IllegalStateException
	{
		InitialContext initialContext = new InitialContext();
		initialContext.bind("testname", "MyFirst");
	}

	@Test
	public void testsInSpecificOrder()
		throws NamingException, MalformedURLException, IllegalStateException
	{
		test1Somewhere();
		test2SomewhereElseEntirely();
		test3ExtraTestWithSpecificContext();
	}

	@Test
	public void test1Somewhere()
		throws NamingException, MalformedURLException, IllegalStateException
	{
		SimpleNamingContextBuilder builder = SimpleNamingContextBuilder.emptyActivatedContextBuilder();
		builder.bind("testname", "WillNotBeCleared");

		InitialContext initialContext = new InitialContext();
		assertEquals("WillNotBeCleared", initialContext.lookup("testname"));
		builder.deactivate();
	}

	@Test
	public void test2SomewhereElseEntirely()
		throws NamingException, MalformedURLException, IllegalStateException
	{
		InitialContext initialContext = new InitialContext();
		assertEquals("MyFirst", initialContext.lookup("testname"));
	}

	@Test
	public void test3ExtraTestWithSpecificContext()
		throws NamingException, MalformedURLException, IllegalStateException
	{
		SimpleNamingContextBuilder builder2 = SimpleNamingContextBuilder.emptyActivatedContextBuilder();
		builder2.bind("testname", "ThisIsNotSet");

		InitialContext initialContext2 = new InitialContext();
		assertEquals("WillNotBeCleared", initialContext2.lookup("testname"));
		builder2.deactivate();
	}

[spring-issuemaster]

Eelco de Lang commented

The deactivate in my workaround is not a no-op. The original context is used after calling it.
Because a new builder can not be set again, it used the stored old builder.

The real problem is that you can easily have two different builder. One on the stack and a different one in the NamingManager. This leads to strange testcan results, that will be depend on the order which they are run.

I see three solutions: 

• Reuse the first builder.
• Throw an exception on any use after a deactivate. 
• Change the builder in the NamingManager through introspection.