Author: awiner
Date: Thu Jul  2 19:36:15 2009
New Revision: 790719

URL: http://svn.apache.org/viewvc?rev=790719&view=rev
Log:
Fix AbstractTagHandler.getValueFromTag() to return null if the attribute isn't 
present.

The existing behavior led to lots of unnecessary expression evaluation, in 
particular unnecessary coercion, though not returning null is kinda handy.  
Added a test for AbstractTagHandler, as well as tests for IfTagHandler and 
RepeatTagHandler that verify behavior when their relevant attributes are 
altogether missing.  And make RepeatTagHandler not depend on 
DefaultTemplateProcessor ignoring null Iterables for repeating.

Added:
    
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/AbstractTagHandlerTest.java
   (with props)
Modified:
    
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/AbstractTagHandler.java
    
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/RepeatTagHandler.java
    
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/IfTagHandlerTest.java
    
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/RepeatTagHandlerTest.java

Modified: 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/AbstractTagHandler.java
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/AbstractTagHandler.java?rev=790719&r1=790718&r2=790719&view=diff
==============================================================================
--- 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/AbstractTagHandler.java
 (original)
+++ 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/AbstractTagHandler.java
 Thu Jul  2 19:36:15 2009
@@ -52,9 +52,25 @@
     return namespaceUri;
   }
 
+  /**
+   * Returns the value of a tag attribute, evaluating any contained EL
+   * expressions if necessary.
+   * 
+   * @param <T> the type of the value
+   * @param tag the element
+   * @param name the attribute name
+   * @param processor the template processor
+   * @param type the type of the value
+   * @return the value of the attribute, or null if the attribute is not
+   *     present.
+   */
   protected final <T> T getValueFromTag(Element tag, String name,
       TemplateProcessor processor, Class<T> type) {
-    return processor.evaluate(tag.getAttribute(name), type, null);
+    if (tag.hasAttribute(name)) {
+      return processor.evaluate(tag.getAttribute(name), type, null);
+    } else {
+      return null;
+    }
   }
 
   protected final DocumentFragment processChildren(Element tag,

Modified: 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/RepeatTagHandler.java
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/RepeatTagHandler.java?rev=790719&r1=790718&r2=790719&view=diff
==============================================================================
--- 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/RepeatTagHandler.java
 (original)
+++ 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/RepeatTagHandler.java
 Thu Jul  2 19:36:15 2009
@@ -40,19 +40,21 @@
 
   public void process(final Node result, final Element tag, final 
TemplateProcessor processor) {
     Iterable<?> repeat = getValueFromTag(tag, EXPRESSION_ATTR, processor, 
Iterable.class);
-    final Attr ifAttribute = tag.getAttributeNode(IF_ATTR);
+    if (repeat != null) {
+      final Attr ifAttribute = tag.getAttributeNode(IF_ATTR);
     
-    // On each iteration, process child nodes, after checking the value of the 
"if" attribute
-    processor.processRepeat(result, tag, repeat, new Runnable() {
-      public void run() {
-        if (ifAttribute != null) {
-          if (!processor.evaluate(ifAttribute.getValue(), Boolean.class, 
false)) {
-            return;
+      // On each iteration, process child nodes, after checking the value of 
the "if" attribute
+      processor.processRepeat(result, tag, repeat, new Runnable() {
+        public void run() {
+          if (ifAttribute != null) {
+            if (!processor.evaluate(ifAttribute.getValue(), Boolean.class, 
false)) {
+              return;
+            }
           }
-        }
 
-        processor.processChildNodes(result, tag);
-      }
-    });
+          processor.processChildNodes(result, tag);
+        }
+      });
+    }
   }
 }

Added: 
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/AbstractTagHandlerTest.java
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/AbstractTagHandlerTest.java?rev=790719&view=auto
==============================================================================
--- 
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/AbstractTagHandlerTest.java
 (added)
+++ 
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/AbstractTagHandlerTest.java
 Thu Jul  2 19:36:15 2009
@@ -0,0 +1,79 @@
+/*
+ * 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 org.apache.shindig.gadgets.templates;
+
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.eq;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.isNull;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+import org.apache.shindig.gadgets.parse.ParseModule;
+import org.junit.Before;
+import org.junit.Test;
+import org.w3c.dom.DOMImplementation;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.Node;
+
+public class AbstractTagHandlerTest {
+  private DOMImplementation documentProvider;
+  private Document document;
+  private AbstractTagHandler handler;
+  private TemplateProcessor templateProcessor;
+
+
+  @Before
+  public void setUp() throws Exception {
+    documentProvider = new ParseModule.DOMImplementationProvider().get();
+    document = documentProvider.createDocument(null, null, null);
+    handler = new AbstractTagHandler(null, null) {
+      public void process(Node result, Element tag, TemplateProcessor 
processor) {
+      }      
+    };
+    
+    templateProcessor = createMock(TemplateProcessor.class);
+  }
+  
+  @Test
+  public void getValueFromTag() {
+    Element element = document.createElement("test");
+    element.setAttribute("key", "expression");
+ 
+    expect(templateProcessor.evaluate(eq("expression"), eq(String.class), 
(String) isNull()))
+        .andReturn("evaluated");
+    replay(templateProcessor);
+    
+    assertEquals("evaluated",
+        handler.getValueFromTag(element, "key", templateProcessor, 
String.class));
+    verify(templateProcessor);
+  }
+  
+  @Test
+  public void getValueFromTagNoAttribute() {
+    Element element = document.createElement("test");
+
+    replay(templateProcessor);
+    assertNull(handler.getValueFromTag(element, "notthere", templateProcessor, 
String.class));
+    verify(templateProcessor);
+  }  
+}

Propchange: 
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/AbstractTagHandlerTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: 
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/IfTagHandlerTest.java
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/IfTagHandlerTest.java?rev=790719&r1=790718&r2=790719&view=diff
==============================================================================
--- 
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/IfTagHandlerTest.java
 (original)
+++ 
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/IfTagHandlerTest.java
 Thu Jul  2 19:36:15 2009
@@ -74,4 +74,16 @@
     handler.process(null, tag, processor);
     verify(processor);
   }
+
+  @Test
+  public void conditionIsMissing() throws Exception {
+    Document doc = documentProvider.createDocument(null, null, null);
+    // Create a mock tag;  the name doesn't truly matter
+    Element tag = doc.createElement("if");
+    
+    replay(processor);
+    handler.process(null, tag, processor);
+    verify(processor);
+  }
+
 }

Modified: 
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/RepeatTagHandlerTest.java
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/RepeatTagHandlerTest.java?rev=790719&r1=790718&r2=790719&view=diff
==============================================================================
--- 
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/RepeatTagHandlerTest.java
 (original)
+++ 
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/templates/RepeatTagHandlerTest.java
 Thu Jul  2 19:36:15 2009
@@ -66,6 +66,17 @@
   }
 
   @Test
+  public void repeatWithoutExpression() throws Exception {
+    Document doc = documentProvider.createDocument(null, null, null);
+    // Create a mock tag;  the name doesn't truly matter
+    Element tag = doc.createElement("repeat");
+    
+    replay(processor);
+    handler.process(null, tag, processor);
+    verify(processor);
+  }
+
+  @Test
   public void repeatWithIf() throws Exception {
     Document doc = documentProvider.createDocument(null, null, null);
     // Create a mock tag;  the name doesn't truly matter


Reply via email to