Repository: incubator-freemarker Updated Branches: refs/heads/3 45a397092 -> dd70af30f
CommonBuilder.builder() now can only be called once. This is to prevents users shooting themselves in the foot, when this way the unwillingly share stateful property value objects between the two built objects (such as share a templateCacheStore object between two Configuration-s). It's especially easy to fall into that trap when the shared property object is a default value. Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/dd70af30 Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/dd70af30 Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/dd70af30 Branch: refs/heads/3 Commit: dd70af30ff0edebebd490cc3ad22a987d6d6c131 Parents: 45a3970 Author: ddekany <[email protected]> Authored: Thu Jun 8 01:43:21 2017 +0200 Committer: ddekany <[email protected]> Committed: Thu Jun 8 01:43:21 2017 +0200 ---------------------------------------------------------------------- .../core/TemplateConfigurationTest.java | 36 ++-- .../DefaultObjectWrapperSingletonsTest.java | 179 +++++++++++-------- .../model/impl/DefaultObjectWrapperTest.java | 5 +- .../apache/freemarker/core/Configuration.java | 19 +- .../freemarker/core/TemplateConfiguration.java | 9 +- .../core/model/impl/ClassIntrospector.java | 15 +- .../core/model/impl/DefaultObjectWrapper.java | 10 +- .../model/impl/RestrictedObjectWrapper.java | 9 +- .../freemarker/core/util/CommonBuilder.java | 5 +- .../core/util/ProductWrappingBuilder.java | 5 + 10 files changed, 182 insertions(+), 110 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/dd70af30/freemarker-core-test/src/test/java/org/apache/freemarker/core/TemplateConfigurationTest.java ---------------------------------------------------------------------- diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/TemplateConfigurationTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/TemplateConfigurationTest.java index eb5e7da..a3fe6dc 100644 --- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/TemplateConfigurationTest.java +++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/TemplateConfigurationTest.java @@ -731,17 +731,18 @@ public class TemplateConfigurationTest { @Test public void testInterpret() throws TemplateException, IOException { - TemplateConfiguration.Builder tcb = new TemplateConfiguration.Builder(); - tcb.setArithmeticEngine(new DummyArithmeticEngine()); { - TemplateConfiguration tc = tcb.build(); + TemplateConfiguration tc = new TemplateConfiguration.Builder() + .arithmeticEngine(new DummyArithmeticEngine()) + .build(); assertOutputWithoutAndWithTC(tc, "<#setting locale='en_US'><#assign src = r'${1} <#assign x = 1>${x + x}'><@src?interpret />", "1 2", "11 22"); } - tcb.setWhitespaceStripping(false); { - TemplateConfiguration tc = tcb.build(); + TemplateConfiguration tc = new TemplateConfiguration.Builder() + .whitespaceStripping(false) + .build(); assertOutputWithoutAndWithTC(tc, "<#if true>\nX</#if><#assign src = r'<#if true>\nY</#if>'><@src?interpret />", "XY", "\nX\nY"); @@ -763,15 +764,15 @@ public class TemplateConfigurationTest { } { - TemplateConfiguration.Builder tcb = new TemplateConfiguration.Builder(); Charset outputEncoding = ISO_8859_2; - tcb.setOutputEncoding(outputEncoding); String legacyNCFtl = "${r'.output_encoding!\"null\"'?eval}"; String camelCaseNCFtl = "${r'.outputEncoding!\"null\"'?eval}"; { - TemplateConfiguration tc = tcb.build(); + TemplateConfiguration tc = new TemplateConfiguration.Builder() + .outputEncoding(outputEncoding) + .build(); // Default is re-auto-detecting in ?eval: assertOutputWithoutAndWithTC(tc, legacyNCFtl, "null", outputEncoding.name()); @@ -779,20 +780,20 @@ public class TemplateConfigurationTest { } { - // Force camelCase: - tcb.setNamingConvention(NamingConvention.CAMEL_CASE); - - TemplateConfiguration tc = tcb.build(); + TemplateConfiguration tc = new TemplateConfiguration.Builder() + .outputEncoding(outputEncoding) + .namingConvention(NamingConvention.CAMEL_CASE) // Force camelCase + .build(); assertOutputWithoutAndWithTC(tc, legacyNCFtl, "null", null); assertOutputWithoutAndWithTC(tc, camelCaseNCFtl, "null", outputEncoding.name()); } { - // Force legacy: - tcb.setNamingConvention(NamingConvention.LEGACY); - - TemplateConfiguration tc = tcb.build(); + TemplateConfiguration tc = new TemplateConfiguration.Builder() + .outputEncoding(outputEncoding) + .namingConvention(NamingConvention.LEGACY) // Force legacy + .build(); assertOutputWithoutAndWithTC(tc, legacyNCFtl, "null", outputEncoding.name()); assertOutputWithoutAndWithTC(tc, camelCaseNCFtl, "null", null); @@ -840,8 +841,9 @@ public class TemplateConfigurationTest { public void testIsSet() throws Exception { for (PropertyDescriptor pd : getTemplateConfigurationSettingPropDescs( TemplateConfiguration.Builder.class, true)) { + checkAllIsSetFalseExcept(new TemplateConfiguration.Builder().build(), null); + TemplateConfiguration.Builder tcb = new TemplateConfiguration.Builder(); - checkAllIsSetFalseExcept(tcb.build(), null); pd.getWriteMethod().invoke(tcb, SETTING_ASSIGNMENTS.get(pd.getName())); checkAllIsSetFalseExcept(tcb.build(), pd.getName()); } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/dd70af30/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperSingletonsTest.java ---------------------------------------------------------------------- diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperSingletonsTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperSingletonsTest.java index d6f8177..ae21f2d 100644 --- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperSingletonsTest.java +++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperSingletonsTest.java @@ -161,10 +161,12 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { } { - DefaultObjectWrapper.Builder factory = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); - factory.setExposureLevel(DefaultObjectWrapper.EXPOSE_PROPERTIES_ONLY); - DefaultObjectWrapper ow1 = factory.build(); - DefaultObjectWrapper ow2 = factory.build(); + DefaultObjectWrapper ow1 = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .exposureLevel(DefaultObjectWrapper.EXPOSE_PROPERTIES_ONLY) + .build(); + DefaultObjectWrapper ow2 = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .exposureLevel(DefaultObjectWrapper.EXPOSE_PROPERTIES_ONLY) + .build(); assertEquals(3, getDefaultObjectWrapperInstanceCacheSize()); assertSame(ow1, ow2); @@ -179,10 +181,12 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { } { - DefaultObjectWrapper.Builder factory = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); - factory.setExposeFields(true); - DefaultObjectWrapper ow1 = factory.build(); - DefaultObjectWrapper ow2 = factory.build(); + DefaultObjectWrapper ow1 = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .exposeFields(true) + .build(); + DefaultObjectWrapper ow2 = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .exposeFields(true) + .build(); assertEquals(4, getDefaultObjectWrapperInstanceCacheSize()); assertSame(ow1, ow2); @@ -194,11 +198,11 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { } { - DefaultObjectWrapper.Builder factory = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); - factory.setStrict(true); - factory.setDefaultDateType(TemplateDateModel.DATETIME); - factory.setOuterIdentity(new RestrictedObjectWrapper.Builder(Configuration.VERSION_3_0_0).build()); - DefaultObjectWrapper ow = factory.build(); + DefaultObjectWrapper ow = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .strict(true) + .defaultDateType(TemplateDateModel.DATETIME) + .outerIdentity(new RestrictedObjectWrapper.Builder(Configuration.VERSION_3_0_0).build()) + .build(); assertEquals(5, getDefaultObjectWrapperInstanceCacheSize()); assertTrue(ow.isStrict()); assertEquals(TemplateDateModel.DATETIME, ow.getDefaultDateType()); @@ -233,9 +237,9 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { } { - DefaultObjectWrapper.Builder factory = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); - factory.setUseModelCache(true); - DefaultObjectWrapper ow = factory.build(); + DefaultObjectWrapper ow = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .useModelCache(true) + .build(); assertTrue(ow.getUseModelCache()); assertEquals(2, getDefaultObjectWrapperInstanceCacheSize()); @@ -246,9 +250,7 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { } private DefaultObjectWrapper getDefaultObjectWrapperWithSetting(Version ici, boolean useModelCache) { - DefaultObjectWrapper.Builder f = new DefaultObjectWrapper.Builder(ici); - f.setUseModelCache(useModelCache); - return f.build(); + return new DefaultObjectWrapper.Builder(ici).useModelCache(useModelCache).build(); } public void testMultipleTCCLs() { @@ -317,17 +319,15 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { checkClassIntrospectorCacheSize(0); List<DefaultObjectWrapper> hardReferences = new LinkedList<>(); - DefaultObjectWrapper.Builder builder; { - builder = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); - - DefaultObjectWrapper bw1 = builder.build(); + DefaultObjectWrapper bw1 = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0).build(); checkClassIntrospectorCacheSize(1); - builder.setExposureLevel(DefaultObjectWrapper.EXPOSE_SAFE); // this was already set to this - builder.setUseModelCache(true); // this shouldn't matter for the introspection cache - DefaultObjectWrapper bw2 = builder.build(); + DefaultObjectWrapper bw2 = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .exposureLevel(DefaultObjectWrapper.EXPOSE_SAFE) // this was already set to this + .useModelCache(true) // this shouldn't matter for the introspection cache + .build(); checkClassIntrospectorCacheSize(1); assertSame(bw2.getClassIntrospector(), bw1.getClassIntrospector()); @@ -345,9 +345,9 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { } { - builder = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); - builder.setExposeFields(true); - DefaultObjectWrapper ow = builder.build(); + DefaultObjectWrapper ow = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .exposeFields(true) + .build(); checkClassIntrospectorCacheSize(2); // Wrapping tests: assertTrue(exposesFields(ow)); @@ -359,8 +359,10 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { } { - builder.setExposureLevel(DefaultObjectWrapper.EXPOSE_ALL); - DefaultObjectWrapper ow = builder.build(); + DefaultObjectWrapper ow = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .exposureLevel(DefaultObjectWrapper.EXPOSE_ALL) + .exposeFields(true) + .build(); checkClassIntrospectorCacheSize(3); // Wrapping tests: assertTrue(exposesFields(ow)); @@ -372,8 +374,9 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { } { - builder.setExposeFields(false); - DefaultObjectWrapper ow = builder.build(); + DefaultObjectWrapper ow = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .exposureLevel(DefaultObjectWrapper.EXPOSE_ALL) + .build(); checkClassIntrospectorCacheSize(4); // Wrapping tests: assertFalse(exposesFields(ow)); @@ -385,8 +388,9 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { } { - builder.setExposureLevel(DefaultObjectWrapper.EXPOSE_NOTHING); - DefaultObjectWrapper ow = builder.build(); + DefaultObjectWrapper ow = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .exposureLevel(DefaultObjectWrapper.EXPOSE_NOTHING) + .build(); checkClassIntrospectorCacheSize(5); // Wrapping tests: assertFalse(exposesFields(ow)); @@ -398,8 +402,10 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { } { - builder.setExposeFields(true); - DefaultObjectWrapper ow = builder.build(); + DefaultObjectWrapper ow = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .exposureLevel(DefaultObjectWrapper.EXPOSE_NOTHING) + .exposeFields(true) + .build(); checkClassIntrospectorCacheSize(6); // Wrapping tests: assertTrue(exposesFields(ow)); @@ -411,8 +417,10 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { } { - builder.setExposureLevel(DefaultObjectWrapper.EXPOSE_PROPERTIES_ONLY); - DefaultObjectWrapper ow = builder.build(); + DefaultObjectWrapper ow = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .exposureLevel(DefaultObjectWrapper.EXPOSE_PROPERTIES_ONLY) + .exposeFields(true) + .build(); checkClassIntrospectorCacheSize(7); // Wrapping tests: assertTrue(exposesFields(ow)); @@ -424,17 +432,19 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { } { - builder = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); - builder.setUseModelCache(true); - builder.setExposeFields(false); - builder.setExposureLevel(DefaultObjectWrapper.EXPOSE_PROPERTIES_ONLY); - - DefaultObjectWrapper bw1 = builder.build(); + DefaultObjectWrapper bw1 = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .useModelCache(true) + .exposeFields(false) + .exposureLevel(DefaultObjectWrapper.EXPOSE_PROPERTIES_ONLY) + .build(); checkClassIntrospectorCacheSize(8); ClassIntrospector ci1 = bw1.getClassIntrospector(); - builder.setUseModelCache(false); // Shouldn't mater for the ClassIntrospector - DefaultObjectWrapper bw2 = builder.build(); + DefaultObjectWrapper bw2 = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .useModelCache(false) // Shouldn't mater for the ClassIntrospector + .exposeFields(false) + .exposureLevel(DefaultObjectWrapper.EXPOSE_PROPERTIES_ONLY) + .build(); ClassIntrospector ci2 = bw2.getClassIntrospector(); checkClassIntrospectorCacheSize(8); @@ -460,15 +470,18 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { assertEquals(0, getClassIntrospectorNonClearedInstanceCacheSize()); { - builder.setExposeFields(false); - - DefaultObjectWrapper bw1 = builder.build(); + DefaultObjectWrapper bw1 = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .exposureLevel(DefaultObjectWrapper.EXPOSE_PROPERTIES_ONLY) + .build(); checkClassIntrospectorCacheSize(8); assertEquals(1, getClassIntrospectorNonClearedInstanceCacheSize()); ClassIntrospector ci1 = bw1.getClassIntrospector(); - builder.setUseModelCache(true); // Shouldn't mater - DefaultObjectWrapper bw2 = builder.build(); + DefaultObjectWrapper bw2 = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .exposureLevel(DefaultObjectWrapper.EXPOSE_PROPERTIES_ONLY) + .useModelCache(true) // Shouldn't mater for the ClassIntrospector + .build(); + ClassIntrospector ci2 = bw2.getClassIntrospector(); assertSame(ci1, ci2); @@ -486,8 +499,8 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { } { - builder = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); - DefaultObjectWrapper ow = builder.build(); + DefaultObjectWrapper ow = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .build(); checkClassIntrospectorCacheSize(8); assertEquals(2, getClassIntrospectorNonClearedInstanceCacheSize()); // Wrapping tests: @@ -504,9 +517,9 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { assertEquals(0, getClassIntrospectorNonClearedInstanceCacheSize()); { - builder = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); - builder.setExposeFields(true); - DefaultObjectWrapper ow = builder.build(); + DefaultObjectWrapper ow = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .exposeFields(true) + .build(); checkClassIntrospectorCacheSize(1); // Wrapping tests: assertTrue(exposesFields(ow)); @@ -518,19 +531,26 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { } { - builder = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); - builder.setMethodAppearanceFineTuner(new MethodAppearanceFineTuner() { + MethodAppearanceFineTuner methodAppearanceFineTuner = new MethodAppearanceFineTuner() { @Override public void process(DecisionInput in, Decision out) { } - }); // spoils ClassIntrospector() sharing - - builder.setUseModelCache(false); - DefaultObjectWrapper bw1 = builder.build(); - assertSame(bw1, builder.build()); - - builder.setUseModelCache(true); - DefaultObjectWrapper bw2 = builder.build(); + }; + DefaultObjectWrapper bw1 = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .useModelCache(false) + .methodAppearanceFineTuner(methodAppearanceFineTuner) // spoils ClassIntrospector() sharing + .build(); + assertSame( + bw1, + new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .useModelCache(false) + .methodAppearanceFineTuner(methodAppearanceFineTuner) // spoils ClassIntrospector() sharing + .build()); + + DefaultObjectWrapper bw2 = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .useModelCache(true) + .methodAppearanceFineTuner(methodAppearanceFineTuner) // spoils ClassIntrospector() sharing + .build(); checkClassIntrospectorCacheSize(1); assertNotSame(bw1, bw2); assertNotSame(bw1.getClassIntrospector(), bw2.getClassIntrospector()); @@ -539,17 +559,22 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase { } { - builder = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); - builder.setMethodAppearanceFineTuner( - GetlessMethodsAsPropertyGettersRule.INSTANCE); // doesn't spoils sharing - - builder.setUseModelCache(false); - DefaultObjectWrapper bw1 = builder.build(); - assertSame(bw1, builder.build()); + DefaultObjectWrapper bw1 = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .methodAppearanceFineTuner(GetlessMethodsAsPropertyGettersRule.INSTANCE) // doesn't spoils sharing + .useModelCache(false) + .build(); + assertSame( + bw1, + new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .methodAppearanceFineTuner(GetlessMethodsAsPropertyGettersRule.INSTANCE) // doesn't spoils sharing + .useModelCache(false) + .build()); checkClassIntrospectorCacheSize(2); - - builder.setUseModelCache(true); - DefaultObjectWrapper bw2 = builder.build(); + + DefaultObjectWrapper bw2 = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0) + .methodAppearanceFineTuner(GetlessMethodsAsPropertyGettersRule.INSTANCE) // doesn't spoils sharing + .useModelCache(true) + .build(); checkClassIntrospectorCacheSize(2); assertNotSame(bw1, bw2); http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/dd70af30/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperTest.java ---------------------------------------------------------------------- diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperTest.java index 221b7b3..6581b10 100644 --- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperTest.java +++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperTest.java @@ -129,9 +129,8 @@ public class DefaultObjectWrapperTest { @SuppressWarnings("boxing") @Test public void testBuilder() throws Exception { - DefaultObjectWrapper.Builder builder = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); - DefaultObjectWrapper ow = builder.build(); - assertSame(ow, builder.build()); + DefaultObjectWrapper ow = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0).build(); + assertSame(ow, new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0).build()); assertSame(ow.getClass(), DefaultObjectWrapper.class); assertEquals(Configuration.VERSION_3_0_0, ow.getIncompatibleImprovements()); } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/dd70af30/freemarker-core/src/main/java/org/apache/freemarker/core/Configuration.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/Configuration.java b/freemarker-core/src/main/java/org/apache/freemarker/core/Configuration.java index 150f9d1..dc86aae 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/Configuration.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/Configuration.java @@ -1807,11 +1807,14 @@ public final class Configuration implements TopLevelConfiguration, CustomStateSc private Collection<OutputFormat> registeredCustomOutputFormats; private Map<String, Object> sharedVariables; + private boolean alreadyBuilt; + /** * @param incompatibleImprovements - * The inital value of the {@link Configuration#getIncompatibleImprovements() incompatibleImprovements}; - * can't {@code null}. This can be later changed via {@link #setIncompatibleImprovements(Version)}. The - * point here is just to ensure that it's never {@code null}. + * The initial value of the + * {@link Configuration#getIncompatibleImprovements() incompatibleImprovements}; can't {@code null}. + * This can be later changed via {@link #setIncompatibleImprovements(Version)}. The point here is + * just to ensure that it's never {@code null}. */ protected ExtendableBuilder(Version incompatibleImprovements) { _NullArgumentException.check("incompatibleImprovements", incompatibleImprovements); @@ -1820,7 +1823,12 @@ public final class Configuration implements TopLevelConfiguration, CustomStateSc @Override public Configuration build() throws ConfigurationException { - return new Configuration(this); + if (alreadyBuilt) { + throw new IllegalStateException("build() can only be executed once."); + } + Configuration configuration = new Configuration(this); + alreadyBuilt = true; + return configuration; } @Override @@ -1839,7 +1847,8 @@ public final class Configuration implements TopLevelConfiguration, CustomStateSc } else { setSourceEncoding(Charset.forName(value)); } - } else if (LOCALIZED_TEMPLATE_LOOKUP_KEY_SNAKE_CASE.equals(name) || LOCALIZED_TEMPLATE_LOOKUP_KEY_CAMEL_CASE.equals(name)) { + } else if (LOCALIZED_TEMPLATE_LOOKUP_KEY_SNAKE_CASE.equals(name) + || LOCALIZED_TEMPLATE_LOOKUP_KEY_CAMEL_CASE.equals(name)) { setLocalizedTemplateLookup(_StringUtil.getYesNo(value)); } else if (WHITESPACE_STRIPPING_KEY_SNAKE_CASE.equals(name) || WHITESPACE_STRIPPING_KEY_CAMEL_CASE.equals(name)) { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/dd70af30/freemarker-core/src/main/java/org/apache/freemarker/core/TemplateConfiguration.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/TemplateConfiguration.java b/freemarker-core/src/main/java/org/apache/freemarker/core/TemplateConfiguration.java index f5d7457..40542a6 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/TemplateConfiguration.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/TemplateConfiguration.java @@ -664,13 +664,20 @@ public final class TemplateConfiguration implements ParsingAndProcessingConfigur public static final class Builder extends MutableParsingAndProcessingConfiguration<Builder> implements CommonBuilder<TemplateConfiguration> { + private boolean alreadyBuilt; + public Builder() { super(); } @Override public TemplateConfiguration build() { - return new TemplateConfiguration(this); + if (alreadyBuilt) { + throw new IllegalStateException("build() can only be executed once."); + } + TemplateConfiguration templateConfiguration = new TemplateConfiguration(this); + alreadyBuilt = true; + return templateConfiguration; } @Override http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/dd70af30/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/ClassIntrospector.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/ClassIntrospector.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/ClassIntrospector.java index 348aab8..0b9129e 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/ClassIntrospector.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/ClassIntrospector.java @@ -1088,6 +1088,8 @@ class ClassIntrospector { // default can be left unset (like null). // - If you add a new field, review all methods in this class, also the ClassIntrospector constructor + private boolean alreadyBuilt; + Builder(ClassIntrospector ci) { exposureLevel = ci.exposureLevel; exposeFields = ci.exposeFields; @@ -1230,10 +1232,14 @@ class ClassIntrospector { */ @Override public ClassIntrospector build() { + if (alreadyBuilt) { + throw new IllegalStateException("build() can only be executed once."); + } + + ClassIntrospector instance; if ((methodAppearanceFineTuner == null || methodAppearanceFineTuner instanceof SingletonCustomizer) && (methodSorter == null || methodSorter instanceof SingletonCustomizer)) { // Instance can be cached. - ClassIntrospector instance; synchronized (INSTANCE_CACHE) { Reference instanceRef = (Reference) INSTANCE_CACHE.get(this); instance = instanceRef != null ? (ClassIntrospector) instanceRef.get() : null; @@ -1245,14 +1251,15 @@ class ClassIntrospector { } removeClearedReferencesFromInstanceCache(); - - return instance; } else { // If methodAppearanceFineTuner or methodSorter is specified and isn't marked as a singleton, the // ClassIntrospector can't be shared/cached as those objects could contain a back-reference to the // DefaultObjectWrapper. - return new ClassIntrospector(this, new Object(), true, false); + instance = new ClassIntrospector(this, new Object(), true, false); } + + alreadyBuilt = true; + return instance; } } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/dd70af30/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapper.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapper.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapper.java index 0efe315..e9b04ee 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapper.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapper.java @@ -1303,6 +1303,8 @@ public class DefaultObjectWrapper implements RichObjectWrapper { INSTANCE_CACHE = new WeakHashMap<>(); private final static ReferenceQueue<DefaultObjectWrapper> INSTANCE_CACHE_REF_QUEUE = new ReferenceQueue<>(); + private boolean alreadyBuilt; + /** * See {@link ExtendableBuilder#ExtendableBuilder(Version, boolean)} */ @@ -1323,8 +1325,14 @@ public class DefaultObjectWrapper implements RichObjectWrapper { */ @Override public DefaultObjectWrapper build() { - return DefaultObjectWrapperTCCLSingletonUtil.getSingleton( + if (alreadyBuilt) { + throw new IllegalStateException("build() can only be executed once."); + } + + DefaultObjectWrapper singleton = DefaultObjectWrapperTCCLSingletonUtil.getSingleton( this, INSTANCE_CACHE, INSTANCE_CACHE_REF_QUEUE, ConstructorInvoker.INSTANCE); + alreadyBuilt = true; + return singleton; } /** http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/dd70af30/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/RestrictedObjectWrapper.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/RestrictedObjectWrapper.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/RestrictedObjectWrapper.java index 43d47c0..12ac9e0 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/RestrictedObjectWrapper.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/RestrictedObjectWrapper.java @@ -71,6 +71,7 @@ public class RestrictedObjectWrapper extends DefaultObjectWrapper { INSTANCE_CACHE = new WeakHashMap<>(); private final static ReferenceQueue<RestrictedObjectWrapper> INSTANCE_CACHE_REF_QUEUE = new ReferenceQueue<>(); + private boolean alreadyBuilt; public Builder(Version incompatibleImprovements) { super(incompatibleImprovements, false); @@ -78,8 +79,14 @@ public class RestrictedObjectWrapper extends DefaultObjectWrapper { @Override public RestrictedObjectWrapper build() { - return DefaultObjectWrapperTCCLSingletonUtil.getSingleton( + if (alreadyBuilt) { + throw new IllegalStateException("build() can only be executed once."); + } + + RestrictedObjectWrapper singleton = DefaultObjectWrapperTCCLSingletonUtil.getSingleton( this, INSTANCE_CACHE, INSTANCE_CACHE_REF_QUEUE, ConstructorInvoker.INSTANCE); + alreadyBuilt = true; + return singleton; } private static class ConstructorInvoker http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/dd70af30/freemarker-core/src/main/java/org/apache/freemarker/core/util/CommonBuilder.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/util/CommonBuilder.java b/freemarker-core/src/main/java/org/apache/freemarker/core/util/CommonBuilder.java index 11aab33..b2c3c2d 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/util/CommonBuilder.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/util/CommonBuilder.java @@ -28,7 +28,10 @@ public interface CommonBuilder<ProductT> { /** * Creates an instance of the product class. This is usually a new instance, though if the product is stateless, - * it's possibly a shared object instead of a new one. + * it's possibly a shared object instead of a new one. Builders shouldn't allow calling this method for multiple + * times (not counting calls that threw exceptions), and should throw {@link IllegalStateException} to prevent that. + * + * @thorws IllegalStateException If this method has already returned successfully once. */ ProductT build() throws ConfigurationException; http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/dd70af30/freemarker-core/src/main/java/org/apache/freemarker/core/util/ProductWrappingBuilder.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/util/ProductWrappingBuilder.java b/freemarker-core/src/main/java/org/apache/freemarker/core/util/ProductWrappingBuilder.java index 4b76dc5..86d0b3e 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/util/ProductWrappingBuilder.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/util/ProductWrappingBuilder.java @@ -25,6 +25,7 @@ package org.apache.freemarker.core.util; public class ProductWrappingBuilder<ProductT> implements CommonBuilder<ProductT> { private final ProductT product; + private boolean alreadyBuilt; public ProductWrappingBuilder(ProductT product) { _NullArgumentException.check("product", product); @@ -33,6 +34,10 @@ public class ProductWrappingBuilder<ProductT> implements CommonBuilder<ProductT> @Override public ProductT build() { + if (alreadyBuilt) { + throw new IllegalStateException("build() can only be executed once."); + } + alreadyBuilt = true; return product; } }
