You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@cocoon.apache.org by vg...@apache.org on 2007/09/25 03:55:24 UTC

svn commit: r579032 - in /cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main: java/org/apache/cocoon/components/treeprocessor/variables/ resources/META-INF/cocoon/spring/

Author: vgritsenko
Date: Mon Sep 24 18:55:23 2007
New Revision: 579032

URL: http://svn.apache.org/viewvc?rev=579032&view=rev
Log:
Fix NullPointerException.
Remove some overcomponentization: NOPVariableResolver and PreparedVariableResolver
never meant to be components.
Cleanup.

Removed:
    cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/resources/META-INF/cocoon/spring/NOPVariableResolver.xml
    cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/resources/META-INF/cocoon/spring/PreparedVariableResolver.xml
Modified:
    cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/LegacySitemapStringTemplateParser.java
    cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/LegacySubstitutions.java
    cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/NOPVariableResolver.java
    cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/PreparedVariableResolver.java
    cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/StringTemplateParserVariableResolver.java
    cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/VariableResolver.java
    cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/VariableResolverFactory.java

Modified: cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/LegacySitemapStringTemplateParser.java
URL: http://svn.apache.org/viewvc/cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/LegacySitemapStringTemplateParser.java?rev=579032&r1=579031&r2=579032&view=diff
==============================================================================
--- cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/LegacySitemapStringTemplateParser.java (original)
+++ cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/LegacySitemapStringTemplateParser.java Mon Sep 24 18:55:23 2007
@@ -23,7 +23,6 @@
 import java.util.List;
 import java.util.Map;
 
-import org.apache.avalon.framework.service.ServiceException;
 import org.apache.avalon.framework.service.ServiceManager;
 import org.apache.cocoon.components.treeprocessor.InvokeContext;
 import org.apache.cocoon.el.objectmodel.ObjectModel;
@@ -32,10 +31,13 @@
 import org.apache.cocoon.template.expression.AbstractStringTemplateParser;
 import org.apache.commons.io.IOUtils;
 
+/**
+ * @version $Id$
+ */
 public class LegacySitemapStringTemplateParser extends AbstractStringTemplateParser {
     
     private ServiceManager serviceManager;
-    
+
     public ServiceManager getServiceManager() {
         return serviceManager;
     }
@@ -53,17 +55,12 @@
         return substitutions;
     }
     
-public final class SitemapExpressionSubstitution implements Subst {
+    public final class SitemapExpressionSubstitution implements Subst {
         
         private PreparedVariableResolver resolver;
         
-        public SitemapExpressionSubstitution(String expression, ServiceManager serviceManager) throws PatternException {
-            try {
-                this.resolver = (PreparedVariableResolver)serviceManager.lookup(PreparedVariableResolver.ROLE);
-                this.resolver.setExpression(expression);
-            } catch (ServiceException e) {
-                throw new PatternException("Could not obtain PreparedVariableResolver", e);
-            }
+        private SitemapExpressionSubstitution(String expression, ServiceManager manager) throws PatternException {
+            this.resolver = new PreparedVariableResolver(expression, manager);
         }
 
         public Boolean getBooleanValue(ObjectModel objectModel) throws Exception {
@@ -110,5 +107,4 @@
             //ignore
         }   
     }
-
 }

Modified: cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/LegacySubstitutions.java
URL: http://svn.apache.org/viewvc/cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/LegacySubstitutions.java?rev=579032&r1=579031&r2=579032&view=diff
==============================================================================
--- cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/LegacySubstitutions.java (original)
+++ cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/LegacySubstitutions.java Mon Sep 24 18:55:23 2007
@@ -27,33 +27,41 @@
 import org.xml.sax.SAXException;
 import org.xml.sax.SAXParseException;
 
+/**
+ * @version $Id$
+ */
 public class LegacySubstitutions extends Substitutions {
-    
+
     public LegacySubstitutions(LegacySitemapStringTemplateParser stringTemplateParser, Locator location, String stringTemplate) throws SAXException {
         super(stringTemplateParser, location, stringTemplate);
     }
-    
+
     public String toString(Locator location, ObjectModel objectModel) throws SAXException {
         throw new UnsupportedOperationException();
     }
-    
+
     public String toString(Locator location, ObjectModel objectModel, InvokeContext context, Map oldObjectModel) throws SAXParseException {
         StringBuffer buf = new StringBuffer();
-        Iterator iterSubst = iterator();
-        while (iterSubst.hasNext()) {
-            Subst subst = (Subst) iterSubst.next();
+
+        Iterator i = iterator();
+        while (i.hasNext()) {
+            Subst subst = (Subst) i.next();
+
             Object val;
             try {
                 if (subst instanceof LegacySitemapStringTemplateParser.SitemapExpressionSubstitution)
-                    val = ((LegacySitemapStringTemplateParser.SitemapExpressionSubstitution)subst).getStringValue(context, oldObjectModel);
+                    val = ((LegacySitemapStringTemplateParser.SitemapExpressionSubstitution) subst).getStringValue(context, oldObjectModel);
                 else
                     val = subst.getValue(objectModel);
             } catch (Exception e) {
                 throw new SAXParseException(e.getMessage(), location, e);
             }
-            buf.append(val != null ? val.toString() : "");
+
+            if (val != null) {
+                buf.append(val.toString());
+            }
         }
+        
         return buf.toString();
     }
-
 }

Modified: cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/NOPVariableResolver.java
URL: http://svn.apache.org/viewvc/cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/NOPVariableResolver.java?rev=579032&r1=579031&r2=579032&view=diff
==============================================================================
--- cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/NOPVariableResolver.java (original)
+++ cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/NOPVariableResolver.java Mon Sep 24 18:55:23 2007
@@ -27,15 +27,19 @@
  */
 public class NOPVariableResolver extends VariableResolver {
     
-    public final static String ROLE = NOPVariableResolver.class.getName();
+    private String expression;
+
 
-    private String expression = null;
-    
     public NOPVariableResolver() {
-        super("");
+        super();
+    }
+
+    public NOPVariableResolver(String expression) {
+        super();
+        setExpression(expression);
     }
 
-    public void  setExpression(String expression) {
+    public void setExpression(String expression) {
         this.originalExpr = expression;
         if (expression != null) {
             this.expression = VariableResolverFactory.unescape(expression);
@@ -44,9 +48,5 @@
 
     public final String resolve(InvokeContext context, Map objectModel) {
         return this.expression;
-    }
-    
-    public final void release() {
-        // Nothing to do
     }
 }

Modified: cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/PreparedVariableResolver.java
URL: http://svn.apache.org/viewvc/cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/PreparedVariableResolver.java?rev=579032&r1=579031&r2=579032&view=diff
==============================================================================
--- cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/PreparedVariableResolver.java (original)
+++ cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/PreparedVariableResolver.java Mon Sep 24 18:55:23 2007
@@ -40,14 +40,11 @@
  *
  * @version $Id$
  */
-final public class PreparedVariableResolver extends VariableResolver implements Disposable {
+final public class PreparedVariableResolver extends VariableResolver
+                                            implements Disposable {
     
-    public static final String ROLE = PreparedVariableResolver.class.getName();
-
-    private ServiceManager manager;
-    protected List tokens;
-    protected boolean needsMapStack;
-
+    private static final int ROOT_SITEMAP_VARIABLE = 0;
+    private static final int ANCHOR_VAR = -1;
     private static final int OPEN = -2;
     private static final int CLOSE = -3;
     private static final int COLON = -4;
@@ -56,14 +53,26 @@
     private static final int SITEMAP_VAR = -9;
     private static final int THREADSAFE_MODULE = -10;
     private static final int STATEFUL_MODULE = -11;
-    private static final int ROOT_SITEMAP_VARIABLE = 0;
-    private static final int ANCHOR_VAR = -1;
 
     protected static final Token COLON_TOKEN = new Token(COLON);
     protected static final Token OPEN_TOKEN = new Token(OPEN);
     protected static final Token CLOSE_TOKEN = new Token(CLOSE);
     protected static final Token EMPTY_TOKEN = new Token(EXPR);
     
+    private ServiceManager manager;
+    protected List tokens;
+    protected boolean needsMapStack;
+
+
+    public PreparedVariableResolver() {
+        super();
+    }
+
+    public PreparedVariableResolver(String expression, ServiceManager manager) throws PatternException {
+        setManager(manager);
+        setExpression(expression);
+    }
+
     public ServiceManager getManager() {
         return manager;
     }
@@ -71,10 +80,6 @@
     public void setManager(ServiceManager manager) {
         this.manager = manager;
     }
-    
-    public PreparedVariableResolver() {
-        super("");
-    }
 
     public void setExpression(String expr) throws PatternException {
         this.originalExpr = expr;
@@ -399,7 +404,7 @@
 
         public String getStringValue() {
             if (value instanceof String) {
-                return (String)this.value;
+                return (String) this.value;
             }
             return null;
         }
@@ -410,7 +415,7 @@
 
         public boolean equals(Object o) {
             if (o instanceof Token) {
-                return ((Token)o).hasType(this.type);
+                return ((Token) o).hasType(this.type);
             }
             return false;
         }
@@ -421,10 +426,9 @@
 
         public InputModule getModule() {
             if (value instanceof InputModule) {
-                return (InputModule)value;
+                return (InputModule) value;
             }
             return null;
         }
     }
-    
 }

Modified: cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/StringTemplateParserVariableResolver.java
URL: http://svn.apache.org/viewvc/cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/StringTemplateParserVariableResolver.java?rev=579032&r1=579031&r2=579032&view=diff
==============================================================================
--- cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/StringTemplateParserVariableResolver.java (original)
+++ cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/StringTemplateParserVariableResolver.java Mon Sep 24 18:55:23 2007
@@ -30,7 +30,8 @@
  *
  * @version $Id$
  */
-public final class StringTemplateParserVariableResolver extends VariableResolver implements Disposable {
+public final class StringTemplateParserVariableResolver extends VariableResolver
+                                                        implements Disposable {
     
     public final static String ROLE = StringTemplateParserVariableResolver.class.getName();
     
@@ -38,10 +39,11 @@
     private ObjectModel objectModel;
     
     private Substitutions substitutions;
-    
+
+
     public StringTemplateParserVariableResolver() {
-        super("");
-    };
+        super();
+    }
     
     public StringTemplateParser getStringTemplateParser() {
         return stringTemplateParser;
@@ -63,7 +65,7 @@
         this.originalExpr = expression;
         try {
             if (stringTemplateParser instanceof LegacySitemapStringTemplateParser)
-                this.substitutions = new LegacySubstitutions((LegacySitemapStringTemplateParser)stringTemplateParser, null, expression);
+                this.substitutions = new LegacySubstitutions((LegacySitemapStringTemplateParser) stringTemplateParser, null, expression);
             else 
                 this.substitutions = new Substitutions(stringTemplateParser, null, expression);
         } catch (Exception e) {
@@ -74,7 +76,7 @@
     public String resolve(InvokeContext context, Map objectModel) throws PatternException {
         try {
             if (this.substitutions instanceof LegacySubstitutions)
-                return ((LegacySubstitutions)substitutions).toString(null, this.objectModel, context, objectModel);
+                return ((LegacySubstitutions) substitutions).toString(null, this.objectModel, context, objectModel);
             else
                 return substitutions.toString(null, this.objectModel);
         } catch (Exception e) {
@@ -85,5 +87,4 @@
     public void dispose() {
         //nothing to do
     }
-
 }

Modified: cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/VariableResolver.java
URL: http://svn.apache.org/viewvc/cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/VariableResolver.java?rev=579032&r1=579031&r2=579032&view=diff
==============================================================================
--- cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/VariableResolver.java (original)
+++ cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/VariableResolver.java Mon Sep 24 18:55:23 2007
@@ -28,6 +28,8 @@
 import org.apache.cocoon.util.location.Locatable;
 import org.apache.cocoon.util.location.Location;
 
+import org.apache.commons.lang.ObjectUtils;
+
 /**
  * Utility class for handling {...} pattern substitutions in sitemap statements.
  *
@@ -35,14 +37,16 @@
  */
 public abstract class VariableResolver {
 
-    public static final Map EMPTY_MAP = Collections.unmodifiableMap(new java.util.HashMap(0));
-
     protected String originalExpr;
+
+
+    protected VariableResolver() {
+    }
     
     protected VariableResolver(String expr) {
         this.originalExpr = expr;
     }
-    
+
     public abstract void setExpression(String expression) throws PatternException;
 
     public final String toString() {
@@ -53,13 +57,12 @@
      * Compare two VariableResolvers
      */
     public boolean equals(Object object) {
-        if (object instanceof VariableResolver) {
-            VariableResolver other = (VariableResolver)object;
-            return (this.originalExpr == null && other.originalExpr == null) ||
-                   (this.originalExpr.equals(other.originalExpr));
-        } else {
+        //noinspection SimplifiableIfStatement
+        if (!(object instanceof VariableResolver)) {
             return false;
         }
+
+        return ObjectUtils.equals(this.originalExpr, ((VariableResolver) object).originalExpr);
     }
 
     /**
@@ -91,12 +94,11 @@
     public static Parameters buildParameters(Map expressions, InvokeContext context, Map objectModel) throws PatternException {
         Location location;
         if (expressions instanceof Locatable) {
-            location = ((Locatable)expressions).getLocation();
+            location = ((Locatable) expressions).getLocation();
         } else {
             location = Location.UNKNOWN;
         }
-        
-        if ((expressions == null || expressions.size() == 0) && location.equals(Location.UNKNOWN)) {
+        if (expressions == null || expressions.size() == 0 && location.equals(Location.UNKNOWN)) {
             return Parameters.EMPTY_PARAMETERS;
         }
 
@@ -104,10 +106,10 @@
 
         Iterator iter = expressions.entrySet().iterator();
         while (iter.hasNext()) {
-            Map.Entry entry = (Map.Entry)iter.next();
+            Map.Entry entry = (Map.Entry) iter.next();
             result.setParameter(
-                ((VariableResolver)entry.getKey()).resolve(context, objectModel),
-                ((VariableResolver)entry.getValue()).resolve(context, objectModel)
+                    ((VariableResolver) entry.getKey()).resolve(context, objectModel),
+                    ((VariableResolver) entry.getValue()).resolve(context, objectModel)
             );
         }
 
@@ -123,12 +125,12 @@
     public static Map buildMap(Map expressions, InvokeContext context, Map objectModel) throws PatternException {
         int size;
         if (expressions == null || (size = expressions.size()) == 0) {
-            return EMPTY_MAP;
+            return Collections.EMPTY_MAP;
         }
 
         Map result;
-        if ( expressions instanceof Locatable ) {
-            result = new SitemapParameters.LocatedHashMap(((Locatable)expressions).getLocation(), size);   
+        if (expressions instanceof Locatable) {
+            result = new SitemapParameters.LocatedHashMap(((Locatable) expressions).getLocation(), size);
         } else {
             result = new HashMap(size);
         }
@@ -137,23 +139,11 @@
         while (iter.hasNext()) {
             Map.Entry entry = (Map.Entry)iter.next();
             result.put(
-                ((VariableResolver)entry.getKey()).resolve(context, objectModel),
-                ((VariableResolver)entry.getValue()).resolve(context, objectModel)
+                    ((VariableResolver) entry.getKey()).resolve(context, objectModel),
+                    ((VariableResolver) entry.getValue()).resolve(context, objectModel)
             );
         }
 
         return result;
     }
-
-//    /**
-//     * Release a <code>Map</code> of expressions.
-//     */
-//    public static void release(Map expressions) {
-//        Iterator iter = expressions.entrySet().iterator();
-//        while (iter.hasNext()) {
-//            Map.Entry entry = (Map.Entry)iter.next();
-//            ((VariableResolver)entry.getKey()).release();
-//            ((VariableResolver)entry.getValue()).release();
-//        }
-//    }
 }

Modified: cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/VariableResolverFactory.java
URL: http://svn.apache.org/viewvc/cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/VariableResolverFactory.java?rev=579032&r1=579031&r2=579032&view=diff
==============================================================================
--- cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/VariableResolverFactory.java (original)
+++ cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main/java/org/apache/cocoon/components/treeprocessor/variables/VariableResolverFactory.java Mon Sep 24 18:55:23 2007
@@ -24,7 +24,6 @@
 import java.util.List;
 
 /**
- *
  * @version $Id$
  */
 public class VariableResolverFactory {
@@ -105,25 +104,21 @@
         if (needsResolve(expression)) {
             VariableResolver resolver;
             try {
-                resolver = (VariableResolver)manager.lookup(StringTemplateParserVariableResolver.ROLE);
+                resolver = (VariableResolver) manager.lookup(StringTemplateParserVariableResolver.ROLE);
                 resolver.setExpression(expression);
             } catch (ServiceException e) {
                 throw new PatternException("Couldn't obtain VariableResolver.", e);
             }
-            List collector = (List)disposableCollector.get();
-            if (collector != null)
+
+            List collector = (List) disposableCollector.get();
+            if (collector != null) {
                 collector.add(resolver);
+            }
 
             return resolver;
 
         }
-        VariableResolver resolver;
-        try {
-            resolver = (VariableResolver)manager.lookup(NOPVariableResolver.ROLE);
-        } catch (ServiceException e) {
-            throw new PatternException("Couldn't obtain VariableResolver.", e);
-        }
-        resolver.setExpression(expression);
-        return resolver;
+
+        return new NOPVariableResolver(expression);
     }
 }



Re: svn commit: r579032 - in /cocoon/trunk/core/cocoon-sitemap/cocoon-sitemap-impl/src/main: java/org/apache/cocoon/components/treeprocessor/variables/ resources/META-INF/cocoon/spring/

Posted by Grzegorz Kossakowski <gk...@apache.org>.
vgritsenko@apache.org pisze:
> Author: vgritsenko
> Date: Mon Sep 24 18:55:23 2007
> New Revision: 579032
> 
> URL: http://svn.apache.org/viewvc?rev=579032&view=rev
> Log:
> Fix NullPointerException.
> Remove some overcomponentization: NOPVariableResolver and PreparedVariableResolver
> never meant to be components.
> Cleanup.

Thanks for this work. If you would like to spend some more time on this stuff take a look at this
comment:
https://issues.apache.org/jira/browse/COCOON-2134#action_12529139

and feel free to assign this issue to yourself.

-- 
Grzegorz Kossakowski
Committer and PMC Member of Apache Cocoon
http://reflectingonthevicissitudes.wordpress.com/