You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@freemarker.apache.org by dd...@apache.org on 2019/12/21 20:14:46 UTC
[freemarker] 01/02: ClassIntrospector related cleanups,
mostly from ported from 2.3
This is an automated email from the ASF dual-hosted git repository.
ddekany pushed a commit to branch 3
in repository https://gitbox.apache.org/repos/asf/freemarker.git
commit ffd4a5f1f821e33afd889017da8ab81b8a6c27a0
Author: ddekany <dd...@apache.org>
AuthorDate: Mon Dec 16 10:34:46 2019 +0100
ClassIntrospector related cleanups, mostly from ported from 2.3
---
.../core/model/impl/DefaultObjectWrapperDesc.java | 2 +-
.../core/model/impl/DefaultObjectWrapperInc.java | 2 +-
.../impl/DefaultObjectWrapperSingletonsTest.java | 20 +++--
.../core/model/impl/DefaultObjectWrapperTest.java | 2 +-
.../impl/Java7MembersOnlyDefaultObjectWrapper.java | 2 +-
.../SimpleMapAndCollectionObjectWrapper.java | 2 +-
.../core/model/impl/ClassIntrospector.java | 98 ++++++++++++----------
.../core/model/impl/DefaultObjectWrapper.java | 48 +++--------
.../core/model/impl/RestrictedObjectWrapper.java | 6 +-
9 files changed, 87 insertions(+), 95 deletions(-)
diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperDesc.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperDesc.java
index 0ed6e7c..b0b02bf 100644
--- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperDesc.java
+++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperDesc.java
@@ -25,7 +25,7 @@ public class DefaultObjectWrapperDesc extends DefaultObjectWrapper {
public DefaultObjectWrapperDesc() {
super(new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0)
- .methodSorter(new AlphabeticalMethodSorter(true)), true);
+ .methodSorter(new AlphabeticalMethodSorter(true)));
}
}
diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperInc.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperInc.java
index eb2cda0..c678f4a 100644
--- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperInc.java
+++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperInc.java
@@ -25,7 +25,7 @@ public class DefaultObjectWrapperInc extends DefaultObjectWrapper {
public DefaultObjectWrapperInc() {
super(new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0)
- .methodSorter(new AlphabeticalMethodSorter(false)), true);
+ .methodSorter(new AlphabeticalMethodSorter(false)));
}
}
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 164ce7d..932dfca 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
@@ -309,11 +309,21 @@ public class DefaultObjectWrapperSingletonsTest extends TestCase {
}
public void testClassInrospectorCache() throws TemplateException {
- assertFalse(new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0)
- .usePrivateCaches(true).build().isClassIntrospectionCacheRestricted());
- assertTrue(new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0)
- .build().isClassIntrospectionCacheRestricted());
-
+ {
+ DefaultObjectWrapper ow = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0)
+ .usePrivateCaches(true)
+ .build();
+ assertFalse(ow.isClassIntrospectionCacheRestricted());
+ assertFalse(ow.getClassIntrospector().isShared());
+ }
+
+ {
+ DefaultObjectWrapper ow = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0)
+ .build();
+ assertTrue(ow.isClassIntrospectionCacheRestricted());
+ assertTrue(ow.getClassIntrospector().isShared());
+ }
+
ClassIntrospector.Builder.clearInstanceCache();
DefaultObjectWrapper.Builder.clearInstanceCache();
checkClassIntrospectorCacheSize(0);
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 d2b37f6..97a46f4 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
@@ -846,7 +846,7 @@ public class DefaultObjectWrapperTest {
private static class CustomizedDefaultObjectWrapper extends DefaultObjectWrapper {
private CustomizedDefaultObjectWrapper(Version incompatibleImprovements) {
- super(new DefaultObjectWrapper.Builder(incompatibleImprovements), true);
+ super(new DefaultObjectWrapper.Builder(incompatibleImprovements));
}
@Override
diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/Java7MembersOnlyDefaultObjectWrapper.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/Java7MembersOnlyDefaultObjectWrapper.java
index 96c1adf..ff602f1 100644
--- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/Java7MembersOnlyDefaultObjectWrapper.java
+++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/Java7MembersOnlyDefaultObjectWrapper.java
@@ -83,7 +83,7 @@ public class Java7MembersOnlyDefaultObjectWrapper extends DefaultObjectWrapper {
};
public Java7MembersOnlyDefaultObjectWrapper(Version version) {
- super(new DefaultObjectWrapper.Builder(version).methodAppearanceFineTuner(POST_JAVA_7_FILTER), true);
+ super(new DefaultObjectWrapper.Builder(version).methodAppearanceFineTuner(POST_JAVA_7_FILTER));
}
private static <T> Set<T> newHashSet(T... items) {
diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/templatesuite/models/SimpleMapAndCollectionObjectWrapper.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/templatesuite/models/SimpleMapAndCollectionObjectWrapper.java
index 8c8dd76..0343770 100644
--- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/templatesuite/models/SimpleMapAndCollectionObjectWrapper.java
+++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/templatesuite/models/SimpleMapAndCollectionObjectWrapper.java
@@ -36,7 +36,7 @@ import org.apache.freemarker.core.model.impl.SimpleSequence;
public class SimpleMapAndCollectionObjectWrapper extends DefaultObjectWrapper {
public SimpleMapAndCollectionObjectWrapper(Version incompatibleImprovements) {
- super(new DefaultObjectWrapper.Builder(incompatibleImprovements), true);
+ super(new DefaultObjectWrapper.Builder(incompatibleImprovements));
}
@Override
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 621f876..951d379 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
@@ -133,8 +133,8 @@ class ClassIntrospector {
final MethodAppearanceFineTuner methodAppearanceFineTuner;
final MethodSorter methodSorter;
- /** See {@link #getHasSharedInstanceRestrictons()} */
- final private boolean hasSharedInstanceRestrictons;
+ /** See {@link #getHasSharedInstanceRestrictions()} */
+ final private boolean hasSharedInstanceRestrictions;
/** See {@link #isShared()} */
final private boolean shared;
@@ -158,22 +158,14 @@ class ClassIntrospector {
// Instantiation:
/**
- * Creates a new instance, that is hence surely not shared (singleton) instance.
- *
- * @param pa
- * Stores what the values of the JavaBean properties of the returned instance will be. Not {@code null}.
- */
- ClassIntrospector(Builder pa, Object sharedLock) {
- this(pa, sharedLock, false, false);
- }
-
- /**
- * @param hasSharedInstanceRestrictons
- * {@code true} exactly if we are creating a new instance with {@link Builder}. Then
- * it's {@code true} even if it won't put the instance into the cache.
+ * @param hasSharedInstanceRestrictions
+ * If the instance should behave as if it is shared, even if it we couldn't put
+ * it into the shared cache.
+ * @param shared
+ * If the instance is in the shared cache, and so is potentially shared.
*/
ClassIntrospector(Builder builder, Object sharedLock,
- boolean hasSharedInstanceRestrictons, boolean shared) {
+ boolean hasSharedInstanceRestrictions, boolean shared) {
_NullArgumentException.check("sharedLock", sharedLock);
exposureLevel = builder.getExposureLevel();
@@ -183,7 +175,7 @@ class ClassIntrospector {
this.sharedLock = sharedLock;
- this.hasSharedInstanceRestrictons = hasSharedInstanceRestrictons;
+ this.hasSharedInstanceRestrictions = hasSharedInstanceRestrictions;
this.shared = shared;
if (CLASS_CHANGE_NOTIFIER != null) {
@@ -191,14 +183,6 @@ class ClassIntrospector {
}
}
- /**
- * Returns a {@link Builder}-s that could be used to invoke an identical {@link #ClassIntrospector}
- * . The returned {@link Builder} can be modified without interfering with anything.
- */
- Builder createBuilder() {
- return new Builder(this);
- }
-
// ------------------------------------------------------------------------------------------------------------------
// Introspection:
@@ -829,7 +813,7 @@ class ClassIntrospector {
* Corresponds to {@link DefaultObjectWrapper#clearClassIntrospectionCache()}.
*/
void clearCache() {
- if (getHasSharedInstanceRestrictons()) {
+ if (getHasSharedInstanceRestrictions()) {
throw new IllegalStateException(
"It's not allowed to clear the whole cache in a read-only " + getClass().getName() +
"instance. Use removeFromClassIntrospectionCache(String prefix) instead.");
@@ -1008,14 +992,14 @@ class ClassIntrospector {
* Returns {@code true} if this instance was created with {@link Builder}, even if it wasn't
* actually put into the cache (as we reserve the right to do so in later versions).
*/
- boolean getHasSharedInstanceRestrictons() {
- return hasSharedInstanceRestrictons;
+ boolean getHasSharedInstanceRestrictions() {
+ return hasSharedInstanceRestrictions;
}
/**
* Tells if this instance is (potentially) shared among {@link DefaultObjectWrapper} instances.
*
- * @see #getHasSharedInstanceRestrictons()
+ * @see #getHasSharedInstanceRestrictions()
*/
boolean isShared() {
return shared;
@@ -1041,10 +1025,12 @@ class ClassIntrospector {
static final class Builder implements CommonBuilder<ClassIntrospector>, Cloneable {
- private static final Map/*<PropertyAssignments, Reference<ClassIntrospector>>*/ INSTANCE_CACHE = new HashMap();
- private static final ReferenceQueue INSTANCE_CACHE_REF_QUEUE = new ReferenceQueue();
+ private static final Map<Builder, Reference<ClassIntrospector>> INSTANCE_CACHE = new HashMap<>();
+ private static final ReferenceQueue<ClassIntrospector> INSTANCE_CACHE_REF_QUEUE = new ReferenceQueue<>();
// Properties and their *defaults*:
+ private boolean sharingDisallowed;
+ private boolean shardingDisallowedSet;
private int exposureLevel = DefaultObjectWrapper.EXPOSE_SAFE;
private boolean exposureLevelSet;
private boolean exposeFields;
@@ -1060,13 +1046,6 @@ class ClassIntrospector {
private boolean alreadyBuilt;
- Builder(ClassIntrospector ci) {
- exposureLevel = ci.exposureLevel;
- exposeFields = ci.exposeFields;
- methodAppearanceFineTuner = ci.methodAppearanceFineTuner;
- methodSorter = ci.methodSorter;
- }
-
Builder(Version incompatibleImprovements) {
// Warning: incompatibleImprovements must not affect this object at versions increments where there's no
// change in the DefaultObjectWrapper.normalizeIncompatibleImprovements results. That is, this class may don't react
@@ -1088,6 +1067,7 @@ class ClassIntrospector {
public int hashCode() {
final int prime = 31;
int result = 1;
+ result = prime * result + (sharingDisallowed ? 1231 : 1237);
result = prime * result + (exposeFields ? 1231 : 1237);
result = prime * result + exposureLevel;
result = prime * result + System.identityHashCode(methodAppearanceFineTuner);
@@ -1102,12 +1082,36 @@ class ClassIntrospector {
if (getClass() != obj.getClass()) return false;
Builder other = (Builder) obj;
+ if (sharingDisallowed != other.sharingDisallowed) return false;
if (exposeFields != other.exposeFields) return false;
if (exposureLevel != other.exposureLevel) return false;
if (methodAppearanceFineTuner != other.methodAppearanceFineTuner) return false;
return methodSorter == other.methodSorter;
}
+ public boolean getShareable() {
+ return sharingDisallowed;
+ }
+
+ /**
+ * Can be used to prevent the sharing of the {@link ClassIntrospector} through the global cache. Setting this
+ * to {@code false} doesn't guarantee that it will be shared (as some other setting values can make that
+ * impossible), but setting this to {@code true} guarantees that it won't be shared. Defaults to {@code false}.
+ *
+ * @see DefaultObjectWrapper.ExtendableBuilder#setUsePrivateCaches(boolean)
+ */
+ public void setSharingDisallowed(boolean shareable) {
+ this.sharingDisallowed = shareable;
+ shardingDisallowedSet = true;
+ }
+
+ /**
+ * Tells if the property was explicitly set, as opposed to just holding its default value.
+ */
+ public boolean isShardingDisallowedSet() {
+ return shardingDisallowedSet;
+ }
+
public int getExposureLevel() {
return exposureLevel;
}
@@ -1171,10 +1175,11 @@ class ClassIntrospector {
}
private static void removeClearedReferencesFromInstanceCache() {
- Reference clearedRef;
+ Reference<? extends ClassIntrospector> clearedRef;
while ((clearedRef = INSTANCE_CACHE_REF_QUEUE.poll()) != null) {
synchronized (INSTANCE_CACHE) {
- findClearedRef: for (Iterator it = INSTANCE_CACHE.values().iterator(); it.hasNext(); ) {
+ findClearedRef: for (Iterator<Reference<ClassIntrospector>> it = INSTANCE_CACHE.values().iterator();
+ it.hasNext(); ) {
if (it.next() == clearedRef) {
it.remove();
break findClearedRef;
@@ -1192,7 +1197,7 @@ class ClassIntrospector {
}
/** For unit testing only */
- static Map getInstanceCache() {
+ static Map<Builder, Reference<ClassIntrospector>> getInstanceCache() {
return INSTANCE_CACHE;
}
@@ -1207,16 +1212,17 @@ class ClassIntrospector {
}
ClassIntrospector instance;
- if ((methodAppearanceFineTuner == null || methodAppearanceFineTuner instanceof SingletonCustomizer)
+ if (!sharingDisallowed
+ && (methodAppearanceFineTuner == null || methodAppearanceFineTuner instanceof SingletonCustomizer)
&& (methodSorter == null || methodSorter instanceof SingletonCustomizer)) {
// Instance can be cached.
synchronized (INSTANCE_CACHE) {
- Reference instanceRef = (Reference) INSTANCE_CACHE.get(this);
- instance = instanceRef != null ? (ClassIntrospector) instanceRef.get() : null;
+ Reference<ClassIntrospector> instanceRef = INSTANCE_CACHE.get(this);
+ instance = instanceRef != null ? instanceRef.get() : null;
if (instance == null) {
Builder thisClone = (Builder) clone(); // prevent any aliasing issues
instance = new ClassIntrospector(thisClone, new Object(), true, true);
- INSTANCE_CACHE.put(thisClone, new WeakReference(instance, INSTANCE_CACHE_REF_QUEUE));
+ INSTANCE_CACHE.put(thisClone, new WeakReference<>(instance, INSTANCE_CACHE_REF_QUEUE));
}
}
@@ -1225,7 +1231,7 @@ class ClassIntrospector {
// 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.
- instance = new ClassIntrospector(this, new Object(), true, false);
+ instance = new ClassIntrospector(this, new Object(), !sharingDisallowed, false);
}
alreadyBuilt = true;
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 26d5cc2..e3474b1 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
@@ -172,29 +172,16 @@ public class DefaultObjectWrapper implements RichObjectWrapper {
/**
* Initializes the instance based on the the {@link ExtendableBuilder} specified.
- *
- * @param finalizeConstruction Decides if the construction is finalized now, or the caller will do some more
- * adjustments on the instance and then call {@link #finalizeConstruction()} itself.
*/
- protected DefaultObjectWrapper(ExtendableBuilder<?, ?> builder, boolean finalizeConstruction) {
+ protected DefaultObjectWrapper(ExtendableBuilder<?, ?> builder) {
incompatibleImprovements = builder.getIncompatibleImprovements(); // normalized
defaultDateType = builder.getDefaultDateType();
outerIdentity = builder.getOuterIdentity() != null ? builder.getOuterIdentity() : this;
strict = builder.isStrict();
- if (builder.getUsePrivateCaches()) {
- // As this is not a read-only DefaultObjectWrapper, the classIntrospector will be possibly replaced for a few times,
- // but we need to use the same sharedInrospectionLock forever, because that's what the model factories
- // synchronize on, even during the classIntrospector is being replaced.
- sharedIntrospectionLock = new Object();
- classIntrospector = new ClassIntrospector(builder.classIntrospectorBuilder, sharedIntrospectionLock);
- } else {
- // As this is a read-only DefaultObjectWrapper, the classIntrospector is never replaced, and since it's shared by
- // other DefaultObjectWrapper instances, we use the lock belonging to the shared ClassIntrospector.
- classIntrospector = builder.classIntrospectorBuilder.build();
- sharedIntrospectionLock = classIntrospector.getSharedLock();
- }
+ classIntrospector = builder.classIntrospectorBuilder.build();
+ sharedIntrospectionLock = classIntrospector.getSharedLock();
staticModels = new StaticModels(this);
enumModels = new EnumModels(this);
@@ -227,21 +214,7 @@ public class DefaultObjectWrapper implements RichObjectWrapper {
}
}
- finalizeConstruction();
- }
-
- /**
- * Meant to be called after {@link DefaultObjectWrapper#DefaultObjectWrapper(ExtendableBuilder, boolean)} when
- * its last argument was {@code false}; makes the instance read-only if necessary, then registers the model
- * factories in the class introspector. No further changes should be done after calling this, if
- * {@code writeProtected} was {@code true}.
- */
- protected void finalizeConstruction() {
- // Attention! At this point, the DefaultObjectWrapper must be fully initialized, as when the model factories are
- // registered below, the DefaultObjectWrapper can immediately get concurrent callbacks. That those other threads will
- // see consistent image of the DefaultObjectWrapper is ensured that callbacks are always sync-ed on
- // classIntrospector.sharedLock, and so is classIntrospector.registerModelFactory(...).
-
+ // Do this only when all fields were set, as this may causes calls to this instance:
registerModelFactories();
}
@@ -321,7 +294,7 @@ public class DefaultObjectWrapper implements RichObjectWrapper {
* will not be actually shared because there's no one to share with, but this will {@code true} even then.
*/
public boolean isClassIntrospectionCacheRestricted() {
- return classIntrospector.getHasSharedInstanceRestrictons();
+ return classIntrospector.getHasSharedInstanceRestrictions();
}
private void registerModelFactories() {
@@ -1368,7 +1341,7 @@ public class DefaultObjectWrapper implements RichObjectWrapper {
@Override
public DefaultObjectWrapper invoke(Builder builder) {
- return new DefaultObjectWrapper(builder, true);
+ return new DefaultObjectWrapper(builder);
}
}
@@ -1693,12 +1666,15 @@ public class DefaultObjectWrapper implements RichObjectWrapper {
}
/**
- * Tells if the instance creates should share caches with other {@link DefaultObjectWrapper} instances
- * (where possible), or it should always invoke its own caches and not share that with anyone else.
- * */
+ * Tells if the instance created should share caches with other {@link DefaultObjectWrapper} instances
+ * (where possible), or it should always use its own caches, and not share that with anyone else.
+ * In {@link DefaultObjectWrapper.Builder} this defaults to {@code false}, in order to have a single global
+ * (static) class introspection cache in the JVM.
+ */
public void setUsePrivateCaches(boolean usePrivateCaches) {
this.usePrivateCaches = usePrivateCaches;
usePrivateCachesSet = true;
+ classIntrospectorBuilder.setSharingDisallowed(usePrivateCaches);
}
/**
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 46347c4..1dff3a6 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
@@ -36,8 +36,8 @@ import org.apache.freemarker.core.model.TemplateModel;
*/
public class RestrictedObjectWrapper extends DefaultObjectWrapper {
- protected RestrictedObjectWrapper(Builder builder, boolean finalizeConstruction) {
- super(builder, finalizeConstruction);
+ protected RestrictedObjectWrapper(Builder builder) {
+ super(builder);
}
/**
@@ -97,7 +97,7 @@ public class RestrictedObjectWrapper extends DefaultObjectWrapper {
@Override
public RestrictedObjectWrapper invoke(Builder builder) {
- return new RestrictedObjectWrapper(builder, true);
+ return new RestrictedObjectWrapper(builder);
}
}