Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 6A22A200B12 for ; Sun, 12 Jun 2016 18:53:59 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 6899B160A04; Sun, 12 Jun 2016 16:53:59 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id B70FD160A5C for ; Sun, 12 Jun 2016 18:53:57 +0200 (CEST) Received: (qmail 158 invoked by uid 500); 12 Jun 2016 16:53:57 -0000 Mailing-List: contact notifications-help@freemarker.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@freemarker.incubator.apache.org Delivered-To: mailing list notifications@freemarker.incubator.apache.org Received: (qmail 149 invoked by uid 99); 12 Jun 2016 16:53:56 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 12 Jun 2016 16:53:56 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id 7424EC0DE8 for ; Sun, 12 Jun 2016 16:53:56 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -4.646 X-Spam-Level: X-Spam-Status: No, score=-4.646 tagged_above=-999 required=6.31 tests=[KAM_ASCII_DIVIDERS=0.8, KAM_LAZY_DOMAIN_SECURITY=1, RCVD_IN_DNSWL_HI=-5, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, RP_MATCHES_RCVD=-1.426] autolearn=disabled Received: from mx2-lw-us.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id h9vPXQbW-_3T for ; Sun, 12 Jun 2016 16:53:47 +0000 (UTC) Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx2-lw-us.apache.org (ASF Mail Server at mx2-lw-us.apache.org) with SMTP id B237560DC0 for ; Sun, 12 Jun 2016 16:53:44 +0000 (UTC) Received: (qmail 98111 invoked by uid 99); 12 Jun 2016 16:53:42 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 12 Jun 2016 16:53:42 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 9F0FDED313; Sun, 12 Jun 2016 16:53:42 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: ddekany@apache.org To: notifications@freemarker.incubator.apache.org Date: Sun, 12 Jun 2016 16:53:53 -0000 Message-Id: In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [12/50] incubator-freemarker git commit: Added new setting to DefaultObjectWrapper (and to DefaultObjectWrapperBuilder): interableSupport. This is to fix the issue when you couldn't use #list (or ?has_next, etc.) on a value that only implements Java 5 ja archived-at: Sun, 12 Jun 2016 16:53:59 -0000 Added new setting to DefaultObjectWrapper (and to DefaultObjectWrapperBuilder): interableSupport. This is to fix the issue when you couldn't use #list (or ?has_next, etc.) on a value that only implements Java 5 java.lang.Iterable (not to be confused with Iterator), but not Collection. This is not enabled by default as it's not compatible with some existing templates, most often because they have used someIterable.iterator() in the template as a workaround. When this is enabled, the Iterable object won't be seen as a generic Java object by FreeMarker anymore, and thus just like with Collection-s, its API won't be exposed to the templates. Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/f084603d Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/f084603d Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/f084603d Branch: refs/heads/2.3 Commit: f084603d7d9f76ae5bf0536d2ac6af2041a8db5d Parents: 3c8d41e Author: ddekany Authored: Sat May 28 15:42:43 2016 +0200 Committer: ddekany Committed: Sat May 28 15:42:43 2016 +0200 ---------------------------------------------------------------------- src/main/java/freemarker/core/Configurable.java | 2 +- .../core/NonSequenceOrCollectionException.java | 32 ++++- .../core/_ErrorDescriptionBuilder.java | 8 ++ .../template/DefaultIterableAdapter.java | 89 ++++++++++++ .../template/DefaultIteratorAdapter.java | 4 +- .../freemarker/template/DefaultListAdapter.java | 28 +--- .../DefaultNonListCollectionAdapter.java | 24 +--- .../template/DefaultObjectWrapper.java | 40 +++++- .../DefaultObjectWrapperConfiguration.java | 25 +++- .../DefaultUnassignableIteratorAdapter.java | 52 +++++++ src/manual/en_US/book.xml | 39 ++++-- .../template/DefaultObjectWrapperTest.java | 140 ++++++++++++++++--- 12 files changed, 391 insertions(+), 92 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/f084603d/src/main/java/freemarker/core/Configurable.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/Configurable.java b/src/main/java/freemarker/core/Configurable.java index 7be1e47..ebfb312 100644 --- a/src/main/java/freemarker/core/Configurable.java +++ b/src/main/java/freemarker/core/Configurable.java @@ -1590,7 +1590,7 @@ public class Configurable { *
String value: If the value contains dot, then it's interpreted as an object builder * expression, with the addition that {@link BeansWrapper}, {@link DefaultObjectWrapper} and * {@link SimpleObjectWrapper} can be referred without package name. For example, these strings are valid - * values: {@code "DefaultObjectWrapper(2.3.21)"}, + * values: {@code "DefaultObjectWrapper(2.3.21, forceLegacyNonListCollections=false, iterableSupport=true)"}, * {@code "BeansWrapper(2.3.21, simpleMapWrapper=true)"}. *
If the value does not contain dot, then it must be one of these special values (case insensitive): * {@code "default"} means the default of {@link Configuration} (the default depends on the http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/f084603d/src/main/java/freemarker/core/NonSequenceOrCollectionException.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/NonSequenceOrCollectionException.java b/src/main/java/freemarker/core/NonSequenceOrCollectionException.java index a2d7167..bc172f4 100644 --- a/src/main/java/freemarker/core/NonSequenceOrCollectionException.java +++ b/src/main/java/freemarker/core/NonSequenceOrCollectionException.java @@ -19,9 +19,13 @@ package freemarker.core; +import freemarker.ext.util.WrapperTemplateModel; +import freemarker.template.DefaultObjectWrapper; import freemarker.template.TemplateCollectionModel; import freemarker.template.TemplateModel; import freemarker.template.TemplateSequenceModel; +import freemarker.template.WrappingTemplateModel; +import freemarker.template.utility.CollectionUtils; /** * Indicates that a {@link TemplateSequenceModel} or {@link TemplateCollectionModel} value was expected, but the value @@ -34,6 +38,9 @@ public class NonSequenceOrCollectionException extends UnexpectedTypeException { private static final Class[] EXPECTED_TYPES = new Class[] { TemplateSequenceModel.class, TemplateCollectionModel.class }; + private static final String ITERABLE_SUPPORT_HINT = "The problematic value is a java.lang.Iterable. Using " + + "DefaultObjectWrapper(..., iterableSupport=true) as the object_wrapper setting of the FreeMarker " + + "configuration should solve this."; public NonSequenceOrCollectionException(Environment env) { super(env, "Expecting sequence or collection value here"); @@ -50,19 +57,34 @@ public class NonSequenceOrCollectionException extends UnexpectedTypeException { NonSequenceOrCollectionException( Expression blamed, TemplateModel model, Environment env) throws InvalidReferenceException { - super(blamed, model, "sequence or collection", EXPECTED_TYPES, env); + this(blamed, model, CollectionUtils.EMPTY_OBJECT_ARRAY, env); } NonSequenceOrCollectionException( Expression blamed, TemplateModel model, String tip, Environment env) throws InvalidReferenceException { - super(blamed, model, "sequence or collection", EXPECTED_TYPES, tip, env); + this(blamed, model, new Object[] { tip }, env); } NonSequenceOrCollectionException( - Expression blamed, TemplateModel model, String[] tips, Environment env) throws InvalidReferenceException { - super(blamed, model, "sequence or collection", EXPECTED_TYPES, tips, env); - } + Expression blamed, TemplateModel model, Object[] tips, Environment env) throws InvalidReferenceException { + super(blamed, model, "sequence or collection", EXPECTED_TYPES, extendTipsIfIterable(model, tips), env); + } + + private static Object[] extendTipsIfIterable(TemplateModel model, Object[] tips) { + if (model instanceof WrapperTemplateModel + && ((WrapperTemplateModel) model).getWrappedObject() instanceof Iterable) { + final int tipsLen = tips != null ? tips.length : 0; + Object[] extendedTips = new Object[tipsLen + 1]; + for (int i = 0; i < tipsLen; i++) { + extendedTips[i] = tips[i]; + } + extendedTips[tipsLen] = ITERABLE_SUPPORT_HINT; + return extendedTips; + } else { + return tips; + } + } } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/f084603d/src/main/java/freemarker/core/_ErrorDescriptionBuilder.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/_ErrorDescriptionBuilder.java b/src/main/java/freemarker/core/_ErrorDescriptionBuilder.java index a4b4731..9249f20 100644 --- a/src/main/java/freemarker/core/_ErrorDescriptionBuilder.java +++ b/src/main/java/freemarker/core/_ErrorDescriptionBuilder.java @@ -306,6 +306,10 @@ public class _ErrorDescriptionBuilder { } private _ErrorDescriptionBuilder tip(Object tip) { + if (tip == null) { + return this; + } + if (this.tip == null) { this.tip = tip; } else { @@ -326,6 +330,10 @@ public class _ErrorDescriptionBuilder { } public _ErrorDescriptionBuilder tips(Object... tips) { + if (tips == null || tips.length == 0) { + return this; + } + if (this.tips == null) { this.tips = tips; } else { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/f084603d/src/main/java/freemarker/template/DefaultIterableAdapter.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/template/DefaultIterableAdapter.java b/src/main/java/freemarker/template/DefaultIterableAdapter.java new file mode 100644 index 0000000..46b1e7a --- /dev/null +++ b/src/main/java/freemarker/template/DefaultIterableAdapter.java @@ -0,0 +1,89 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package freemarker.template; + +import java.io.Serializable; +import java.util.Collection; +import java.util.Iterator; +import java.util.List; + +import freemarker.core._DelayedShortClassName; +import freemarker.core._TemplateModelException; +import freemarker.ext.util.WrapperTemplateModel; +import freemarker.template.utility.ObjectWrapperWithAPISupport; + +/** + * Adapts an {@link Iterable} to the corresponding {@link TemplateModel} interface(s), most importantly to + * {@link TemplateCollectionModel}. This should only be used if {@link Collection} is not implemented by the adapted + * object, because then {@link DefaultListAdapter} and {@link DefaultNonListCollectionAdapter} gives more functionality. + * + *

+ * Thread safety: A {@link DefaultIterableAdapter} is as thread-safe as the {@link Iterable} that it wraps is. Normally + * you only have to consider read-only access, as the FreeMarker template language doesn't provide mean to call + * {@link Iterator} modifier methods (though of course, Java methods called from the template can violate this rule). + * + *

+ * This adapter is used by {@link DefaultObjectWrapper} if its {@link DefaultObjectWrapper#setIterableSupport(boolean) + * iterableSupport} property is {@code true}, which is not the default for backward compatibility (so you have to set it + * explicitly). + * + * @since 2.3.25 + */ +@SuppressWarnings("serial") +public class DefaultIterableAdapter extends WrappingTemplateModel implements TemplateCollectionModel, + AdapterTemplateModel, WrapperTemplateModel, TemplateModelWithAPISupport, Serializable { + + private final Iterable iterable; + + /** + * Factory method for creating new adapter instances. + * + * @param iterable + * The collection to adapt; can't be {@code null}. + * @param wrapper + * The {@link ObjectWrapper} used to wrap the items in the array. Has to be + * {@link ObjectWrapperAndUnwrapper} because of planned future features. + */ + public static DefaultIterableAdapter adapt(Iterable iterable, ObjectWrapperWithAPISupport wrapper) { + return new DefaultIterableAdapter(iterable, wrapper); + } + + private DefaultIterableAdapter(Iterable iterable, ObjectWrapperWithAPISupport wrapper) { + super(wrapper); + this.iterable = iterable; + } + + public TemplateModelIterator iterator() throws TemplateModelException { + return new DefaultUnassignableIteratorAdapter(iterable.iterator(), getObjectWrapper()); + } + + public Object getWrappedObject() { + return iterable; + } + + public Object getAdaptedObject(Class hint) { + return getWrappedObject(); + } + + public TemplateModel getAPI() throws TemplateModelException { + return ((ObjectWrapperWithAPISupport) getObjectWrapper()).wrapAsAPI(iterable); + } + +} http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/f084603d/src/main/java/freemarker/template/DefaultIteratorAdapter.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/template/DefaultIteratorAdapter.java b/src/main/java/freemarker/template/DefaultIteratorAdapter.java index a99d5b7..7ca7f31 100644 --- a/src/main/java/freemarker/template/DefaultIteratorAdapter.java +++ b/src/main/java/freemarker/template/DefaultIteratorAdapter.java @@ -27,7 +27,9 @@ import freemarker.ext.util.WrapperTemplateModel; /** * Adapts an {@link Iterator} to the corresponding {@link TemplateModel} interface(s), most importantly to - * {@link TemplateCollectionModel}. The resulting {@link TemplateCollectionModel} can only be iterated once. + * {@link TemplateCollectionModel}. The resulting {@link TemplateCollectionModel} can only be listed (iterated) once. + * If the user tries list the variable for a second time, an exception will be thrown instead of silently gettig an + * empty (or partial) listing. * *

* Thread safety: A {@link DefaultListAdapter} is as thread-safe as the array that it wraps is. Normally you only http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/f084603d/src/main/java/freemarker/template/DefaultListAdapter.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/template/DefaultListAdapter.java b/src/main/java/freemarker/template/DefaultListAdapter.java index 4188d77..39b107f 100644 --- a/src/main/java/freemarker/template/DefaultListAdapter.java +++ b/src/main/java/freemarker/template/DefaultListAdapter.java @@ -21,9 +21,7 @@ package freemarker.template; import java.io.Serializable; import java.util.AbstractSequentialList; -import java.util.Iterator; import java.util.List; -import java.util.NoSuchElementException; import freemarker.ext.util.WrapperTemplateModel; import freemarker.template.utility.ObjectWrapperWithAPISupport; @@ -98,31 +96,7 @@ public class DefaultListAdapter extends WrappingTemplateModel implements Templat } public TemplateModelIterator iterator() throws TemplateModelException { - return new IteratorAdapter(list.iterator(), getObjectWrapper()); - } - - } - - private static class IteratorAdapter implements TemplateModelIterator { - - private final Iterator it; - private final ObjectWrapper wrapper; - - private IteratorAdapter(Iterator it, ObjectWrapper wrapper) { - this.it = it; - this.wrapper = wrapper; - } - - public TemplateModel next() throws TemplateModelException { - try { - return wrapper.wrap(it.next()); - } catch (NoSuchElementException e) { - throw new TemplateModelException("The collection has no more items.", e); - } - } - - public boolean hasNext() throws TemplateModelException { - return it.hasNext(); + return new DefaultUnassignableIteratorAdapter(list.iterator(), getObjectWrapper()); } } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/f084603d/src/main/java/freemarker/template/DefaultNonListCollectionAdapter.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/template/DefaultNonListCollectionAdapter.java b/src/main/java/freemarker/template/DefaultNonListCollectionAdapter.java index ca44395..d14c507 100644 --- a/src/main/java/freemarker/template/DefaultNonListCollectionAdapter.java +++ b/src/main/java/freemarker/template/DefaultNonListCollectionAdapter.java @@ -75,7 +75,7 @@ public class DefaultNonListCollectionAdapter extends WrappingTemplateModel imple } public TemplateModelIterator iterator() throws TemplateModelException { - return new IteratorAdapter(collection.iterator()); + return new DefaultUnassignableIteratorAdapter(collection.iterator(), getObjectWrapper()); } public int size() { @@ -94,28 +94,6 @@ public class DefaultNonListCollectionAdapter extends WrappingTemplateModel imple return getWrappedObject(); } - private class IteratorAdapter implements TemplateModelIterator { - - private final Iterator iterator; - - IteratorAdapter(Iterator iterator) { - this.iterator = iterator; - } - - public TemplateModel next() throws TemplateModelException { - if (!iterator.hasNext()) { - throw new TemplateModelException("The collection has no more items."); - } - - Object value = iterator.next(); - return value instanceof TemplateModel ? (TemplateModel) value : wrap(value); - } - - public boolean hasNext() throws TemplateModelException { - return iterator.hasNext(); - } - } - public boolean contains(TemplateModel item) throws TemplateModelException { Object itemPojo = ((ObjectWrapperAndUnwrapper) getObjectWrapper()).unwrap(item); try { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/f084603d/src/main/java/freemarker/template/DefaultObjectWrapper.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/template/DefaultObjectWrapper.java b/src/main/java/freemarker/template/DefaultObjectWrapper.java index 8753ebb..b2f5820 100644 --- a/src/main/java/freemarker/template/DefaultObjectWrapper.java +++ b/src/main/java/freemarker/template/DefaultObjectWrapper.java @@ -43,11 +43,13 @@ import freemarker.log.Logger; * {@link Configuration#Configuration(Version) incompatibleImprovements} 2.3.22 (or higher). * *

- * If you still need to create an instance, that should be done with an {@link DefaultObjectWrapperBuilder}, not with + * If you still need to create an instance, that should be done with an {@link DefaultObjectWrapperBuilder} (or + * with {@link Configuration#setSetting(String, String)} with {@code "object_wrapper"} key), not with * its constructor, as that allows FreeMarker to reuse singletons. For new projects, it's recommended to set * {@link DefaultObjectWrapperBuilder#setForceLegacyNonListCollections(boolean) forceLegacyNonListCollections} to - * {@code false} - something that setting {@code incompatibleImprovements} to 2.3.22 won't do. - * + * {@code false}, and {@link DefaultObjectWrapperBuilder#setIterableSupport(boolean) iterableSupport} to {@code true}; + * setting {@code incompatibleImprovements} to 2.3.22 won't do these, as they could break legacy templates too easily. + * *

* This class is only thread-safe after you have finished calling its setter methods, and then safely published it (see * JSR 133 and related literature). When used as part of {@link Configuration}, of course it's enough if that was safely @@ -65,6 +67,7 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper { private boolean useAdaptersForContainers; private boolean forceLegacyNonListCollections; + private boolean iterableSupport; /** * Creates a new instance with the incompatible-improvements-version specified in @@ -115,6 +118,7 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper { : new DefaultObjectWrapperConfiguration(bwCfg.getIncompatibleImprovements()) { }; useAdaptersForContainers = dowDowCfg.getUseAdaptersForContainers(); forceLegacyNonListCollections = dowDowCfg.getForceLegacyNonListCollections(); + iterableSupport = dowDowCfg.getIterableSupport(); finalizeConstruction(writeProtected); } @@ -222,6 +226,9 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper { ? (TemplateModel) DefaultIteratorAdapter.adapt((Iterator) obj, this) : (TemplateModel) new SimpleCollection((Iterator) obj, this); } + if (iterableSupport && obj instanceof Iterable) { + return DefaultIterableAdapter.adapt((Iterable) obj, this); + } return handleUnknownType(obj); } @@ -331,6 +338,31 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper { } /** + * Getter pair of {@link #setIterableSupport(boolean)}; see there. + * + * @since 2.3.25 + */ + public boolean getIterableSupport() { + return iterableSupport; + } + + /** + * Specifies whether {@link Iterable}-s (not to be confused with {@link Iterator}-s) that don't implement any other + * recognized Java interfaces (most notably {@link Collection}) will be recognized as listable objects + * ({@link TemplateCollectionModel}-s), or they will be just seen as generic objects (JavaBean-s). Defaults to + * {@code false} for backward compatibility, but in new projects you should set this to {@code true}. Before setting + * this to {@code true} in older projects, check if you have called {@code myIterable.iterator()} directly from any + * templates, because the Java API is only exposed to the templates if the {@link Iterable} is wrapped as generic + * object. + * + * @since 2.3.25 + */ + public void setIterableSupport(boolean iterableSupport) { + checkModifiable(); + this.iterableSupport = iterableSupport; + } + + /** * Returns the lowest version number that is equivalent with the parameter version. * * @since 2.3.22 @@ -359,7 +391,7 @@ public class DefaultObjectWrapper extends freemarker.ext.beans.BeansWrapper { } return "useAdaptersForContainers=" + useAdaptersForContainers + ", forceLegacyNonListCollections=" - + forceLegacyNonListCollections + ", " + bwProps; + + forceLegacyNonListCollections + ", iterableSupport=" + iterableSupport + bwProps; } } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/f084603d/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java b/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java index fbf8f85..cde1eca 100644 --- a/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java +++ b/src/main/java/freemarker/template/DefaultObjectWrapperConfiguration.java @@ -25,7 +25,7 @@ import freemarker.ext.beans.BeansWrapperConfiguration; * Holds {@link DefaultObjectWrapper} configuration settings and defines their defaults. * You will not use this abstract class directly, but concrete subclasses like {@link DefaultObjectWrapperBuilder}. * Unless, you are developing a builder for a custom {@link DefaultObjectWrapper} subclass. In that case, note that - * overriding the {@link #equals} and {@link #hashCode} is important, as these object are used as {@link ObjectWrapper} + * overriding the {@link #equals} and {@link #hashCode} is important, as these objects are used as {@link ObjectWrapper} * singleton lookup keys. * * @since 2.3.22 @@ -34,6 +34,7 @@ public abstract class DefaultObjectWrapperConfiguration extends BeansWrapperConf private boolean useAdaptersForContainers; private boolean forceLegacyNonListCollections; + private boolean iterableSupport; protected DefaultObjectWrapperConfiguration(Version incompatibleImprovements) { super(DefaultObjectWrapper.normalizeIncompatibleImprovementsVersion(incompatibleImprovements), true); @@ -60,6 +61,24 @@ public abstract class DefaultObjectWrapperConfiguration extends BeansWrapperConf public void setForceLegacyNonListCollections(boolean legacyNonListCollectionWrapping) { this.forceLegacyNonListCollections = legacyNonListCollectionWrapping; } + + /** + * See {@link DefaultObjectWrapper#getIterableSupport()}. + * + * @since 2.3.25 + */ + public boolean getIterableSupport() { + return iterableSupport; + } + + /** + * See {@link DefaultObjectWrapper#setIterableSupport(boolean)}. + * + * @since 2.3.25 + */ + public void setIterableSupport(boolean iterableSupport) { + this.iterableSupport = iterableSupport; + } @Override public int hashCode() { @@ -67,6 +86,7 @@ public abstract class DefaultObjectWrapperConfiguration extends BeansWrapperConf final int prime = 31; result = result * prime + (useAdaptersForContainers ? 1231 : 1237); result = result * prime + (forceLegacyNonListCollections ? 1231 : 1237); + result = result * prime + (iterableSupport ? 1231 : 1237); return result; } @@ -75,7 +95,8 @@ public abstract class DefaultObjectWrapperConfiguration extends BeansWrapperConf if (!super.equals(that)) return false; final DefaultObjectWrapperConfiguration thatDowCfg = (DefaultObjectWrapperConfiguration) that; return useAdaptersForContainers == thatDowCfg.getUseAdaptersForContainers() - && forceLegacyNonListCollections == thatDowCfg.forceLegacyNonListCollections; + && forceLegacyNonListCollections == thatDowCfg.forceLegacyNonListCollections + && iterableSupport == thatDowCfg.iterableSupport; } } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/f084603d/src/main/java/freemarker/template/DefaultUnassignableIteratorAdapter.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/template/DefaultUnassignableIteratorAdapter.java b/src/main/java/freemarker/template/DefaultUnassignableIteratorAdapter.java new file mode 100644 index 0000000..383c390 --- /dev/null +++ b/src/main/java/freemarker/template/DefaultUnassignableIteratorAdapter.java @@ -0,0 +1,52 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package freemarker.template; + +import java.util.Iterator; +import java.util.NoSuchElementException; + +/** + * As opposed to {@link DefaultIteratorAdapter}, this simpler {@link Iterator} adapter is used in situations where the + * {@link TemplateModelIterator} won't be assigned to FreeMarker template variables, only used internally by + * {@code #list} or custom Java code. Because of that, it doesn't have to handle the situation where the user tries to + * iterate over the same value twice. + */ +class DefaultUnassignableIteratorAdapter implements TemplateModelIterator { + + private final Iterator it; + private final ObjectWrapper wrapper; + + DefaultUnassignableIteratorAdapter(Iterator it, ObjectWrapper wrapper) { + this.it = it; + this.wrapper = wrapper; + } + + public TemplateModel next() throws TemplateModelException { + try { + return wrapper.wrap(it.next()); + } catch (NoSuchElementException e) { + throw new TemplateModelException("The collection has no more items.", e); + } + } + + public boolean hasNext() throws TemplateModelException { + return it.hasNext(); + } + +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/f084603d/src/manual/en_US/book.xml ---------------------------------------------------------------------- diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml index 22578d0..81715bc 100644 --- a/src/manual/en_US/book.xml +++ b/src/manual/en_US/book.xml @@ -26534,15 +26534,36 @@ TemplateModel x = env.getVariable("x"); // get variable x - Bug fixed (FREEMARKER-18): If you had a JSP custom tag and - an EL function defined in the same TLD with the same name, the - EL function has overwritten the custom tag. This is a bug - introduced in 2.3.23, when EL function support was added. JSP - allows a custom tag and an EL function in the same TLD to have - the same name. In such case now we combine the two into a single - value that is both callable as an user defined directive - (<@my.foo...>) and as a function - (my.f(...)). + Added new setting to + DefaultObjectWrapper (and to + DefaultObjectWrapperBuilder): + interableSupport. This is to fix the issue + when you couldn't use #list (or + ?has_next, etc.) on a value that only + implements Java 5 java.lang.Iterable (not to + be confused with Iterator), but not + Collection. This is not enabled by default as + it's not compatible with some existing templates, most often + because they have used + someIterable.iterator() in the template as a + workaround. When this is enabled, the + Iterable object won't be seen as a generic + Java object by FreeMarker anymore, and thus just like with + Collection-s, its API won't be exposed to the + templates. + + + + Bug fixed (FREEMARKER-18): + If you had a JSP custom tag and an EL function defined in the + same TLD with the same name, the EL function has overwritten the + custom tag. This is a bug introduced in 2.3.23, when EL function + support was added. JSP allows a custom tag and an EL function in + the same TLD to have the same name. In such case now we combine + the two into a single value that is both callable as an user + defined directive (<@my.foo...>) and as + a function (my.f(...)). http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/f084603d/src/test/java/freemarker/template/DefaultObjectWrapperTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/freemarker/template/DefaultObjectWrapperTest.java b/src/test/java/freemarker/template/DefaultObjectWrapperTest.java index f94f2d9..9c722cb 100644 --- a/src/test/java/freemarker/template/DefaultObjectWrapperTest.java +++ b/src/test/java/freemarker/template/DefaultObjectWrapperTest.java @@ -25,6 +25,7 @@ import static org.junit.Assert.*; import java.io.IOException; import java.io.StringReader; +import java.io.StringWriter; import java.util.ArrayList; import java.util.Collections; import java.util.Date; @@ -54,6 +55,7 @@ import com.google.common.collect.ImmutableMap; import freemarker.ext.beans.BeansWrapper; import freemarker.ext.beans.HashAdapter; import freemarker.ext.util.WrapperTemplateModel; +import freemarker.test.templatesuite.models.Listables; public class DefaultObjectWrapperTest { @@ -68,11 +70,16 @@ public class DefaultObjectWrapperTest { OW22NM.setNullModel(NullModel.INSTANCE); } - private final static DefaultObjectWrapper OW22_FUTURE = new DefaultObjectWrapper(Configuration.VERSION_2_3_22); + private final static DefaultObjectWrapper OW22CA = new DefaultObjectWrapper(Configuration.VERSION_2_3_22); static { - OW22_FUTURE.setForceLegacyNonListCollections(false); + OW22CA.setForceLegacyNonListCollections(false); } + private final static DefaultObjectWrapper OW22IS = new DefaultObjectWrapper(Configuration.VERSION_2_3_22); + static { + OW22IS.setIterableSupport(true); + } + @Test public void testIncompatibleImprovementsVersionBreakPoints() throws Exception { List expected = new ArrayList(); @@ -284,6 +291,24 @@ public class DefaultObjectWrapperTest { assertTrue(bw.wrap(new String[] {}) instanceof DefaultArrayAdapter); assertTrue(bw.wrap(new HashSet()) instanceof DefaultNonListCollectionAdapter); } + + { + DefaultObjectWrapperBuilder builder = new DefaultObjectWrapperBuilder(Configuration.getVersion()); + DefaultObjectWrapper bw = builder.build(); + assertSame(bw, builder.build()); + assertFalse(bw.getIterableSupport()); + + assertFalse(bw.wrap(new PureIterable()) instanceof DefaultIterableAdapter); + } + { + DefaultObjectWrapperBuilder builder = new DefaultObjectWrapperBuilder(Configuration.getVersion()); + builder.setIterableSupport(true); + DefaultObjectWrapper bw = builder.build(); + assertSame(bw, builder.build()); + assertTrue(bw.getIterableSupport()); + + assertTrue(bw.wrap(new PureIterable()) instanceof DefaultIterableAdapter); + } } @Test @@ -700,25 +725,25 @@ public class DefaultObjectWrapperTest { set.add("a"); set.add("b"); set.add("c"); - TemplateCollectionModelEx coll = (TemplateCollectionModelEx) OW22_FUTURE.wrap(set); + TemplateCollectionModelEx coll = (TemplateCollectionModelEx) OW22CA.wrap(set); assertTrue(coll instanceof DefaultNonListCollectionAdapter); assertEquals(3, coll.size()); assertFalse(coll.isEmpty()); assertCollectionTMEquals(coll, "a", "b", "c"); - assertTrue(coll.contains(OW22_FUTURE.wrap("a"))); - assertTrue(coll.contains(OW22_FUTURE.wrap("b"))); - assertTrue(coll.contains(OW22_FUTURE.wrap("c"))); - assertTrue(coll.contains(OW22_FUTURE.wrap("c"))); - assertFalse(coll.contains(OW22_FUTURE.wrap("d"))); + assertTrue(coll.contains(OW22CA.wrap("a"))); + assertTrue(coll.contains(OW22CA.wrap("b"))); + assertTrue(coll.contains(OW22CA.wrap("c"))); + assertTrue(coll.contains(OW22CA.wrap("c"))); + assertFalse(coll.contains(OW22CA.wrap("d"))); try { - assertFalse(coll.contains(OW22_FUTURE.wrap(1))); + assertFalse(coll.contains(OW22CA.wrap(1))); fail(); } catch (TemplateModelException e) { assertThat(e.getMessage(), containsString("Integer")); } - assertRoundtrip(OW22_FUTURE, set, DefaultNonListCollectionAdapter.class, TreeSet.class, "[a, b, c]"); + assertRoundtrip(OW22CA, set, DefaultNonListCollectionAdapter.class, TreeSet.class, "[a, b, c]"); assertSizeThroughAPIModel(3, coll); } @@ -728,24 +753,24 @@ public class DefaultObjectWrapperTest { final List list = Collections.singletonList("b"); set.add(list); set.add(null); - TemplateCollectionModelEx coll = (TemplateCollectionModelEx) OW22_FUTURE.wrap(set); + TemplateCollectionModelEx coll = (TemplateCollectionModelEx) OW22CA.wrap(set); TemplateModelIterator it = coll.iterator(); final TemplateModel tm1 = it.next(); - Object obj1 = OW22_FUTURE.unwrap(tm1); + Object obj1 = OW22CA.unwrap(tm1); final TemplateModel tm2 = it.next(); - Object obj2 = OW22_FUTURE.unwrap(tm2); + Object obj2 = OW22CA.unwrap(tm2); assertTrue(obj1 == null || obj2 == null); assertTrue(obj1 != null && obj1.equals(list) || obj2 != null && obj2.equals(list)); assertTrue(tm1 instanceof DefaultListAdapter || tm2 instanceof DefaultListAdapter); List similarList = new ArrayList(); similarList.add("b"); - assertTrue(coll.contains(OW22_FUTURE.wrap(similarList))); - assertTrue(coll.contains(OW22_FUTURE.wrap(null))); - assertFalse(coll.contains(OW22_FUTURE.wrap("a"))); - assertFalse(coll.contains(OW22_FUTURE.wrap(1))); + assertTrue(coll.contains(OW22CA.wrap(similarList))); + assertTrue(coll.contains(OW22CA.wrap(null))); + assertFalse(coll.contains(OW22CA.wrap("a"))); + assertFalse(coll.contains(OW22CA.wrap(1))); - assertRoundtrip(OW22_FUTURE, set, DefaultNonListCollectionAdapter.class, HashSet.class, "[" + obj1 + ", " + assertRoundtrip(OW22CA, set, DefaultNonListCollectionAdapter.class, HashSet.class, "[" + obj1 + ", " + obj2 + "]"); } } @@ -755,14 +780,14 @@ public class DefaultObjectWrapperTest { public void testCollectionAdapterOutOfBounds() throws TemplateModelException { Set set = Collections.singleton(123); - TemplateCollectionModelEx coll = (TemplateCollectionModelEx) OW22_FUTURE.wrap(set); + TemplateCollectionModelEx coll = (TemplateCollectionModelEx) OW22CA.wrap(set); TemplateModelIterator it = coll.iterator(); for (int i = 0; i < 3; i++) { assertTrue(it.hasNext()); } - assertEquals(123, OW22_FUTURE.unwrap(it.next())); + assertEquals(123, OW22CA.unwrap(it.next())); for (int i = 0; i < 3; i++) { assertFalse(it.hasNext()); @@ -894,6 +919,50 @@ public class DefaultObjectWrapperTest { } @Test + public void testIterableSupport() throws TemplateException, IOException { + Iterable iterable = new PureIterable(); + + String listingFTL = "<#list value as x>${x}<#sep>, "; + + { + DefaultObjectWrapper ow = OW22; + TemplateModel tm = ow.wrap(iterable); + assertThat(tm, instanceOf(TemplateHashModel.class)); + assertThat(tm, not(instanceOf(TemplateCollectionModel.class))); + assertThat(tm, not(instanceOf(TemplateSequenceModel.class))); + assertNotNull(((TemplateHashModel) tm).get("iterator")); + + assertTemplateFails(ow, iterable, listingFTL, "iterableSupport"); + } + + { + DefaultObjectWrapper ow = OW22IS; + TemplateModel tm = ow.wrap(iterable); + assertThat(tm, instanceOf(TemplateCollectionModel.class)); + TemplateCollectionModel iterableTM = (TemplateCollectionModel) tm; + + for (int i = 0; i < 2; i++) { + TemplateModelIterator iteratorTM = iterableTM.iterator(); + assertTrue(iteratorTM.hasNext()); + assertEquals("a", ow.unwrap(iteratorTM.next())); + assertTrue(iteratorTM.hasNext()); + assertEquals("b", ow.unwrap(iteratorTM.next())); + assertTrue(iteratorTM.hasNext()); + assertEquals("c", ow.unwrap(iteratorTM.next())); + assertFalse(iteratorTM.hasNext()); + try { + iteratorTM.next(); + fail(); + } catch (TemplateModelException e) { + assertThat(e.getMessage(), containsStringIgnoringCase("no more")); + } + } + + assertTemplateOutput(OW22IS, iterable, listingFTL, "a, b, c"); + } + } + + @Test public void assertCanWrapDOM() throws SAXException, IOException, ParserConfigurationException, TemplateModelException { DocumentBuilder db = DocumentBuilderFactory.newInstance().newDocumentBuilder(); @@ -913,6 +982,37 @@ public class DefaultObjectWrapperTest { assertEquals(expectedSize, r.getAsNumber().intValue()); } + private void assertTemplateOutput(ObjectWrapper objectWrapper, Object value, String ftl, String expectedOutput) + throws TemplateException, IOException { + assertEquals(expectedOutput, processTemplate(objectWrapper, value, ftl)); + } + + private void assertTemplateFails(ObjectWrapper objectWrapper, Object value, String ftl, String expectedMessagePart) + throws TemplateException, IOException { + try { + processTemplate(objectWrapper, value, ftl); + fail(); + } catch (TemplateException e) { + assertThat(e.getMessage(), containsString(expectedMessagePart)); + } + } + + private String processTemplate(ObjectWrapper objectWrapper, Object value, String ftl) + throws TemplateException, IOException { + Configuration cfg = new Configuration(Configuration.VERSION_2_3_24); + cfg.setLogTemplateExceptions(false); + cfg.setObjectWrapper(objectWrapper); + StringWriter out = new StringWriter(); + new Template(null, ftl, cfg).process(ImmutableMap.of("value", value), out); + return out.toString(); + } + + private static final class PureIterable implements Iterable { + public Iterator iterator() { + return ImmutableList.of("a", "b", "c").iterator(); + } + } + public static class RoundtripTesterBean { public Class getClass(Object o) {