From 2159518252061d9c5db488cca8f21aa314bb44d1 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 9 Jan 2019 14:54:14 +0100 Subject: [PATCH 1/5] Improve documentation for DI options in the TestContext framework --- src/docs/asciidoc/testing.adoc | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/docs/asciidoc/testing.adoc b/src/docs/asciidoc/testing.adoc index 2425e0352b2..1162c9abe0a 100644 --- a/src/docs/asciidoc/testing.adoc +++ b/src/docs/asciidoc/testing.adoc @@ -3297,15 +3297,22 @@ is cleared. For further details, see the discussion of `@DirtiesContext` in When you use the `DependencyInjectionTestExecutionListener` (which is configured by default), the dependencies of your test instances are injected from beans in the -application context that you configured with `@ContextConfiguration`. You may use setter -injection, field injection, or both, depending on which annotations you choose and -whether you place them on setter methods or fields. For consistency with the annotation -support introduced in Spring 2.5 and 3.0, you can use Spring's `@Autowired` annotation or -the `@Inject` annotation from JSR 330. - -TIP: The TestContext framework does not instrument the manner in which a test instance is -instantiated. Thus, the use of `@Autowired` or `@Inject` for constructors has no effect -for test classes. +application context that you configured with `@ContextConfiguration` or related +annotations. You may use setter injection, field injection, or both, depending on which +annotations you choose and whether you place them on setter methods or fields. If you are +using JUnit Jupiter you may also optionally use constructor injection (see +<>). For consistency with the annotation support introduced +in Spring 2.5 and 3.0, you can use Spring's `@Autowired` annotation or the `@Inject` +annotation from JSR 330 for field and setter injection. + +TIP: For testing frameworks other than JUnit Jupiter, the TestContext framework does not +participate in instantiation of the test class. Thus, the use of `@Autowired` or +`@Inject` for constructors has no effect for test classes. + +NOTE: Although field injection is discouraged in production code, field injection is +actually quite natural in test code. The rationale for the difference is that you will +never instantiate your test class directly. Consequently, there is no need to be able to +invoke a `public` constructor or setter method on your test class. Because `@Autowired` is used to perform <>, if you have multiple bean definitions of the same type, you cannot rely on this @@ -3313,7 +3320,7 @@ approach for those particular beans. In that case, you can use `@Autowired` in conjunction with `@Qualifier`. As of Spring 3.0, you can also choose to use `@Inject` in conjunction with `@Named`. Alternatively, if your test class has access to its `ApplicationContext`, you can perform an explicit lookup by using (for example) a call to -`applicationContext.getBean("titleRepository")`. +`applicationContext.getBean("titleRepository", TitleRepository.class)`. If you do not want dependency injection applied to your test instances, do not annotate fields or setter methods with `@Autowired` or `@Inject`. Alternatively, you can disable @@ -3329,7 +3336,7 @@ is presented after all sample code listings. [NOTE] ==== The dependency injection behavior in the following code listings is not specific to JUnit -4. The same DI techniques can be used in conjunction with any testing framework. +4. The same DI techniques can be used in conjunction with any supported testing framework. The following examples make calls to static assertion methods, such as `assertNotNull()`, but without prepending the call with `Assert`. In such cases, assume that the method was From cf565067fe5b2f9c503e2653c81ba9512993fc48 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 9 Jan 2019 15:39:56 +0100 Subject: [PATCH 2/5] Document effect of @DirtiesContext when used with constructor injection Issue: SPR-17654 --- src/docs/asciidoc/testing.adoc | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/src/docs/asciidoc/testing.adoc b/src/docs/asciidoc/testing.adoc index 1162c9abe0a..33003c6c7e8 100644 --- a/src/docs/asciidoc/testing.adoc +++ b/src/docs/asciidoc/testing.adoc @@ -3120,9 +3120,10 @@ the underlying context cache, you can set the log level for the In the unlikely case that a test corrupts the application context and requires reloading (for example, by modifying a bean definition or the state of an application object), you can annotate your test class or test method with `@DirtiesContext` (see the discussion of -`@DirtiesContext` in <>). This instructs Spring +`@DirtiesContext` in <>). This instructs Spring to remove the context from the cache and rebuild the application context before running -the next test. Note that support for the `@DirtiesContext` annotation is provided by the +the next test that requires the same application context. Note that support for the +`@DirtiesContext` annotation is provided by the `DirtiesContextBeforeModesTestExecutionListener` and the `DirtiesContextTestExecutionListener`, which are enabled by default. @@ -3287,11 +3288,10 @@ shows this configuration scenario: NOTE: If you use `@DirtiesContext` in a test whose context is configured as part of a context hierarchy, you can use the `hierarchyMode` flag to control how the context cache is cleared. For further details, see the discussion of `@DirtiesContext` in -<> and the +<> and the {api-spring-framework}/test/annotation/DirtiesContext.html[`@DirtiesContext`] javadoc. -- - [[testcontext-fixture-di]] ==== Dependency Injection of Test Fixtures @@ -4382,7 +4382,7 @@ and integration tests and simultaneously reap the benefits of the TestContext fr such as support for loading application contexts, dependency injection of test instances, transactional test method execution, and so on. -Furthermore, thanks to the rich extension API in JUnit Jupiter, Spring can provide the +Furthermore, thanks to the rich extension API in JUnit Jupiter, Spring provides the following features above and beyond the feature set that Spring supports for JUnit 4 and TestNG: @@ -4417,7 +4417,7 @@ The following code listing shows how to configure a test class to use the ---- ==== -Since you can also use annotations in JUnit 5 as meta-annotations, Spring can provide the +Since you can also use annotations in JUnit 5 as meta-annotations, Spring provides the `@SpringJUnitConfig` and `@SpringJUnitWebConfig` composed annotations to simplify the configuration of the test `ApplicationContext` and JUnit Jupiter. @@ -4492,6 +4492,25 @@ Spring assumes the responsibility for resolving _all_ parameters in the construc Consequently, no other `ParameterResolver` registered with JUnit Jupiter can resolve parameters for such a constructor. +[WARNING] +==== +Constructor injection for test classes must not be used in conjunction with JUnit +Jupiter's `@TestInstance(PER_CLASS)` support if `@DirtiesContext` is used to close the +test's `ApplicationContext` before or after test methods. + +The reason is that `@TestInstance(PER_CLASS)` instructs JUnit Jupiter to cache the test +instance between test method invocations. Consequently, the test instance will retain +references to beans that were originally injected from an `ApplicationContext` that has +been subsequently closed. Since the constructor for the test class will only be invoked +once in such scenarios, dependency injection will not occur again, and subsequent tests +will interact with beans from the closed `ApplicationContext` which may result in errors. + +To use `@DirtiesContext` with "before test method" or "after test method" modes in +conjunction with `@TestInstance(PER_CLASS)`, one must configure dependencies from Spring +to be supplied via field or setter injection so that they can be re-injected between test +method invocations. +==== + In the following example, Spring injects the `OrderService` bean from the `ApplicationContext` loaded from `TestConfig.class` into the `OrderServiceIntegrationTests` constructor. From e1e6224acb4fcf0d24f0a1134284752b20d611ec Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 9 Jan 2019 16:37:56 +0100 Subject: [PATCH 3/5] Document effect of preemptive timeouts on transactional tests Issue: SPR-17647 --- src/docs/asciidoc/testing.adoc | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/docs/asciidoc/testing.adoc b/src/docs/asciidoc/testing.adoc index 33003c6c7e8..36ba5b333d0 100644 --- a/src/docs/asciidoc/testing.adoc +++ b/src/docs/asciidoc/testing.adoc @@ -3618,6 +3618,29 @@ caution if Spring-managed or application-managed transactions are configured wit propagation type other than `REQUIRED` or `SUPPORTS` (see the discussion on <> for details). +.Preemptive timeouts and test-managed transactions +[WARNING] +==== +Caution must be taken when using any form of preemptive timeouts from a testing framework +in conjunction with Spring's test-managed transactions. + +Specifically, Spring’s testing support binds transaction state to the current thread (via +a `java.lang.ThreadLocal` variable) _before_ the current test method is invoked. If a +testing framework invokes the current test method in a new thread in order to support a +preemptive timeout, any actions performed within the current test method will _not_ be +invoked within the test-managed transaction. Consequently, the result of any such actions +will not be rolled back with the test-managed transaction. On the contrary, such actions +will be committed to the persistent store -- for example, a relational database -- even +though the test-managed transaction is properly rolled back by Spring. + +Situations in which this can occur include but are not limited to the following. + +* JUnit 4's `@Test(timeout = ...)` support and `TimeOut` rule +* JUnit Jupiter's `assertTimeoutPreemptively(...)` methods in the + `org.junit.jupiter.api.Assertions` class +* TestNG's `@Test(timeOut = ...)` support +==== + [[testcontext-tx-enabling-transactions]] ===== Enabling and Disabling Transactions From 86fb43900efc2d0b526ec1564262b7f14d5a4ca7 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Mon, 14 Jan 2019 11:48:41 +0100 Subject: [PATCH 4/5] Deprecate JiBX marshaller Resolves #22249 --- .../oxm/config/JibxMarshallerBeanDefinitionParser.java | 4 +++- .../java/org/springframework/oxm/jibx/JibxMarshaller.java | 4 +++- .../resources/org/springframework/oxm/config/spring-oxm.xsd | 2 +- .../org/springframework/oxm/jibx/JibxMarshallerTests.java | 5 +++-- .../org/springframework/oxm/jibx/JibxUnmarshallerTests.java | 3 ++- 5 files changed, 12 insertions(+), 6 deletions(-) diff --git a/spring-oxm/src/main/java/org/springframework/oxm/config/JibxMarshallerBeanDefinitionParser.java b/spring-oxm/src/main/java/org/springframework/oxm/config/JibxMarshallerBeanDefinitionParser.java index 1ba7e975b36..753ae4add5e 100644 --- a/spring-oxm/src/main/java/org/springframework/oxm/config/JibxMarshallerBeanDefinitionParser.java +++ b/spring-oxm/src/main/java/org/springframework/oxm/config/JibxMarshallerBeanDefinitionParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,7 +25,9 @@ import org.springframework.beans.factory.xml.AbstractSimpleBeanDefinitionParser; * * @author Arjen Poutsma * @since 3.0 + * @deprecated as of Spring Framework 5.1.5, due to the lack of activity on the JiBX project */ +@Deprecated class JibxMarshallerBeanDefinitionParser extends AbstractSimpleBeanDefinitionParser { @Override diff --git a/spring-oxm/src/main/java/org/springframework/oxm/jibx/JibxMarshaller.java b/spring-oxm/src/main/java/org/springframework/oxm/jibx/JibxMarshaller.java index f74860908cb..9775f8878dd 100644 --- a/spring-oxm/src/main/java/org/springframework/oxm/jibx/JibxMarshaller.java +++ b/spring-oxm/src/main/java/org/springframework/oxm/jibx/JibxMarshaller.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -81,7 +81,9 @@ import org.springframework.util.xml.StaxUtils; * @since 3.0 * @see org.jibx.runtime.IMarshallingContext * @see org.jibx.runtime.IUnmarshallingContext + * @deprecated as of Spring Framework 5.1.5, due to the lack of activity on the JiBX project */ +@Deprecated public class JibxMarshaller extends AbstractMarshaller implements InitializingBean { private static final String DEFAULT_BINDING_NAME = "binding"; diff --git a/spring-oxm/src/main/resources/org/springframework/oxm/config/spring-oxm.xsd b/spring-oxm/src/main/resources/org/springframework/oxm/config/spring-oxm.xsd index 2152a14ebd3..35a96256390 100644 --- a/spring-oxm/src/main/resources/org/springframework/oxm/config/spring-oxm.xsd +++ b/spring-oxm/src/main/resources/org/springframework/oxm/config/spring-oxm.xsd @@ -50,7 +50,7 @@ - Defines a JiBX Marshaller. + Defines a JiBX Marshaller. Deprecated as of Spring Framework 5.1.5! diff --git a/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxMarshallerTests.java b/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxMarshallerTests.java index 32ba19acd91..12951092df3 100644 --- a/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxMarshallerTests.java +++ b/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxMarshallerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,7 +26,7 @@ import org.junit.Test; import org.springframework.oxm.AbstractMarshallerTests; import static org.junit.Assert.*; -import static org.xmlunit.matchers.CompareMatcher.*; +import static org.xmlunit.matchers.CompareMatcher.isSimilarTo; /** * NOTE: These tests fail under Eclipse/IDEA because JiBX binding does not occur by @@ -35,6 +35,7 @@ import static org.xmlunit.matchers.CompareMatcher.*; * @author Arjen Poutsma * @author Sam Brannen */ +@Deprecated public class JibxMarshallerTests extends AbstractMarshallerTests { @BeforeClass diff --git a/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxUnmarshallerTests.java b/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxUnmarshallerTests.java index 39f5a66f000..dec1f4f777b 100644 --- a/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxUnmarshallerTests.java +++ b/spring-oxm/src/test/java/org/springframework/oxm/jibx/JibxUnmarshallerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -34,6 +34,7 @@ import static org.junit.Assert.*; * @author Arjen Poutsma * @author Sam Brannen */ +@Deprecated public class JibxUnmarshallerTests extends AbstractUnmarshallerTests { protected static final String INPUT_STRING_WITH_SPECIAL_CHARACTERS = From 2b65d0e51ff106270871455880e2b1cf427eea1b Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 17 Jan 2019 11:09:48 -0500 Subject: [PATCH 5/5] Fix error when writing empty String to DataBuffer Prior to this commit, `DataBuffer.write` would throw an `IllegalStateException` when called with an empty `String`, since the `CharsetEncoder` would be flushed on an incorrent state. This commit skips entirely the encoding phase for empty `String`. Fixes #22262 --- .../core/io/buffer/DataBuffer.java | 46 ++++++++++--------- .../core/io/buffer/DataBufferTests.java | 12 ++++- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/io/buffer/DataBuffer.java b/spring-core/src/main/java/org/springframework/core/io/buffer/DataBuffer.java index c4efe9ef54a..baf161047b2 100644 --- a/spring-core/src/main/java/org/springframework/core/io/buffer/DataBuffer.java +++ b/spring-core/src/main/java/org/springframework/core/io/buffer/DataBuffer.java @@ -246,30 +246,32 @@ public interface DataBuffer { default DataBuffer write(CharSequence charSequence, Charset charset) { Assert.notNull(charSequence, "CharSequence must not be null"); Assert.notNull(charset, "Charset must not be null"); - CharsetEncoder charsetEncoder = charset.newEncoder() - .onMalformedInput(CodingErrorAction.REPLACE) - .onUnmappableCharacter(CodingErrorAction.REPLACE); - CharBuffer inBuffer = CharBuffer.wrap(charSequence); - int estimatedSize = (int) (inBuffer.remaining() * charsetEncoder.averageBytesPerChar()); - ByteBuffer outBuffer = ensureCapacity(estimatedSize) - .asByteBuffer(writePosition(), writableByteCount()); - while (true) { - CoderResult cr = (inBuffer.hasRemaining() ? - charsetEncoder.encode(inBuffer, outBuffer, true) : CoderResult.UNDERFLOW); - if (cr.isUnderflow()) { - cr = charsetEncoder.flush(outBuffer); - } - if (cr.isUnderflow()) { - break; - } - if (cr.isOverflow()) { - writePosition(outBuffer.position()); - int maximumSize = (int) (inBuffer.remaining() * charsetEncoder.maxBytesPerChar()); - ensureCapacity(maximumSize); - outBuffer = asByteBuffer(writePosition(), writableByteCount()); + if (charSequence.length() != 0) { + CharsetEncoder charsetEncoder = charset.newEncoder() + .onMalformedInput(CodingErrorAction.REPLACE) + .onUnmappableCharacter(CodingErrorAction.REPLACE); + CharBuffer inBuffer = CharBuffer.wrap(charSequence); + int estimatedSize = (int) (inBuffer.remaining() * charsetEncoder.averageBytesPerChar()); + ByteBuffer outBuffer = ensureCapacity(estimatedSize) + .asByteBuffer(writePosition(), writableByteCount()); + while (true) { + CoderResult cr = (inBuffer.hasRemaining() ? + charsetEncoder.encode(inBuffer, outBuffer, true) : CoderResult.UNDERFLOW); + if (cr.isUnderflow()) { + cr = charsetEncoder.flush(outBuffer); + } + if (cr.isUnderflow()) { + break; + } + if (cr.isOverflow()) { + writePosition(outBuffer.position()); + int maximumSize = (int) (inBuffer.remaining() * charsetEncoder.maxBytesPerChar()); + ensureCapacity(maximumSize); + outBuffer = asByteBuffer(writePosition(), writableByteCount()); + } } + writePosition(outBuffer.position()); } - writePosition(outBuffer.position()); return this; } diff --git a/spring-core/src/test/java/org/springframework/core/io/buffer/DataBufferTests.java b/spring-core/src/test/java/org/springframework/core/io/buffer/DataBufferTests.java index 5591e628b70..2b78b16df92 100644 --- a/spring-core/src/test/java/org/springframework/core/io/buffer/DataBufferTests.java +++ b/spring-core/src/test/java/org/springframework/core/io/buffer/DataBufferTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -179,6 +179,16 @@ public class DataBufferTests extends AbstractDataBufferAllocatingTestCase { } } + @Test + public void writeEmptyString() { + DataBuffer buffer = createDataBuffer(1); + buffer.write("", StandardCharsets.UTF_8); + + assertEquals(0, buffer.readableByteCount()); + + release(buffer); + } + @Test public void writeUtf8String() { DataBuffer buffer = createDataBuffer(6);