You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by jo...@apache.org on 2010/02/04 22:13:55 UTC
svn commit: r906650 - in /incubator/shindig/trunk:
features/src/main/javascript/features/globals/
java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/
java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/
java/gadgets/src/main/java/...
Author: johnh
Date: Thu Feb 4 21:13:55 2010
New Revision: 906650
URL: http://svn.apache.org/viewvc?rev=906650&view=rev
Log:
Emit DOMException information in output code rather than simply returning a null which NPEs downstream. Further clean-up may be warranted to return a non-200 status response in this case, but this should be better than status quo.
Modified:
incubator/shindig/trunk/features/src/main/javascript/features/globals/globals.js
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaHtmlParser.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/parse/AbstractSocialMarkupHtmlParserTest.java
Modified: incubator/shindig/trunk/features/src/main/javascript/features/globals/globals.js
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/features/src/main/javascript/features/globals/globals.js?rev=906650&r1=906649&r2=906650&view=diff
==============================================================================
--- incubator/shindig/trunk/features/src/main/javascript/features/globals/globals.js (original)
+++ incubator/shindig/trunk/features/src/main/javascript/features/globals/globals.js Thu Feb 4 21:13:55 2010
@@ -2,10 +2,10 @@
* @namespace The global gadgets namespace
* @type {Object}
*/
-var gadgets = {};
+var gadgets = gadgets || {};
/**
* @namespace The global shindig namespace, used for shindig specific extensions and data
* @type {Object}
*/
-var shindig = {};
+var shindig = shindig || {};
Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java?rev=906650&r1=906649&r2=906650&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java Thu Feb 4 21:13:55 2010
@@ -21,7 +21,6 @@
import org.apache.shindig.common.cache.CacheProvider;
import org.apache.shindig.common.util.HashUtil;
import org.apache.shindig.gadgets.GadgetException;
-import org.apache.shindig.gadgets.GadgetException.Code;
import org.apache.shindig.gadgets.parse.nekohtml.NekoSimplifiedHtmlParser;
import com.google.common.collect.BiMap;
@@ -31,8 +30,10 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import org.w3c.dom.Attr;
+import org.w3c.dom.DOMImplementation;
import org.w3c.dom.Document;
import org.w3c.dom.DocumentFragment;
+import org.w3c.dom.DOMException;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
@@ -50,6 +51,7 @@
private Cache<String, Document> documentCache;
private Cache<String, DocumentFragment> fragmentCache;
private Provider<HtmlSerializer> serializerProvider = new DefaultSerializerProvider();
+ protected final DOMImplementation documentFactory;
/**
* Allowed tag names for OpenSocial Data and template blocks.
@@ -62,6 +64,10 @@
*/
public static final BiMap<String, String> SCRIPT_TYPE_TO_OSML_TAG = ImmutableBiMap.of(
"text/os-data", OSML_DATA_TAG, "text/os-template", OSML_TEMPLATE_TAG);
+
+ protected GadgetHtmlParser(DOMImplementation documentFactory) {
+ this.documentFactory = documentFactory;
+ }
@Inject
public void setCacheProvider(CacheProvider cacheProvider) {
@@ -99,9 +105,9 @@
document = parseDomImpl(source);
} catch (GadgetException e) {
throw e;
- } catch (Exception e) {
+ } catch (DOMException e) {
// DOMException is a RuntimeException
- throw new GadgetException(Code.HTML_PARSE_ERROR, e);
+ return errorDom(e);
}
HtmlSerialization.attach(document, serializerProvider.get(), source);
@@ -240,9 +246,10 @@
DocumentFragment fragment = null;
try {
fragment = parseFragmentImpl(source);
- } catch (Exception e) {
+ } catch (DOMException e) {
// DOMException is a RuntimeException
- throw new GadgetException(Code.HTML_PARSE_ERROR, e);
+ appendParseException(result, e);
+ return;
}
reprocessScriptForOpenSocial(fragment);
@@ -261,6 +268,27 @@
}
}
+ protected Document errorDom(DOMException e) {
+ // Create a bare-bones DOM whose body is just error text.
+ // We do this to echo information to the developer that originally
+ // supplied the data, since doing so is more useful than simply
+ // returning a black-box HTML error code stemming from an NPE or other condition downstream.
+ // The method is protected to allow overriding of this behavior.
+ Document doc = documentFactory.createDocument(null, null, null);
+ Node html = doc.createElement("html");
+ html.appendChild(doc.createElement("head"));
+ Node body = doc.createElement("body");
+ appendParseException(body, e);
+ html.appendChild(body);
+ doc.appendChild(html);
+ return doc;
+ }
+
+ private void appendParseException(Node node, DOMException e) {
+ node.appendChild(node.getOwnerDocument().createTextNode(
+ GadgetException.Code.HTML_PARSE_ERROR.toString() + ": " + e.toString()));
+ }
+
protected boolean shouldCache() {
return documentCache != null && documentCache.getCapacity() != 0;
}
Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaHtmlParser.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaHtmlParser.java?rev=906650&r1=906649&r2=906650&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaHtmlParser.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaHtmlParser.java Thu Feb 4 21:13:55 2010
@@ -40,11 +40,9 @@
import org.w3c.dom.Node;
public class CajaHtmlParser extends GadgetHtmlParser {
- private final DOMImplementation documentFactory;
-
@Inject
public CajaHtmlParser(DOMImplementation documentFactory) {
- this.documentFactory = documentFactory;
+ super(documentFactory);
}
@Override
Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java?rev=906650&r1=906649&r2=906650&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java Thu Feb 4 21:13:55 2010
@@ -88,11 +88,9 @@
private static final Map<String, HTMLElements.Element> OSML_ELEMENTS = ImmutableMap.of(
OSML_TEMPLATE_TAG, OSML_TEMPLATE_ELEMENT, OSML_DATA_TAG, OSML_DATA_ELEMENT);
- private final DOMImplementation documentFactory;
-
@Inject
public NekoSimplifiedHtmlParser(DOMImplementation documentFactory) {
- this.documentFactory = documentFactory;
+ super(documentFactory);
}
@Override
Modified: incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/parse/AbstractSocialMarkupHtmlParserTest.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/parse/AbstractSocialMarkupHtmlParserTest.java?rev=906650&r1=906649&r2=906650&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/parse/AbstractSocialMarkupHtmlParserTest.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/parse/AbstractSocialMarkupHtmlParserTest.java Thu Feb 4 21:13:55 2010
@@ -20,14 +20,12 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import org.apache.commons.lang.StringUtils;
import org.apache.shindig.common.xml.DomUtil;
-import org.apache.shindig.gadgets.GadgetException;
import org.apache.shindig.gadgets.parse.GadgetHtmlParser;
import org.apache.shindig.gadgets.parse.HtmlSerialization;
import org.apache.shindig.gadgets.spec.PipelinedData;
@@ -35,7 +33,6 @@
import org.junit.Before;
import org.junit.Test;
import org.w3c.dom.Attr;
-import org.w3c.dom.DOMException;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
@@ -149,16 +146,14 @@
public void testInvalid() throws Exception {
String content =
"<html><div id=\"div_super\" class=\"div_super\" valign:\"middle\"></div></html>";
- try {
- parser.parseDom(content);
- fail("No exception caught on invalid character");
- } catch (DOMException e) {
- assertTrue(e.getMessage().contains("INVALID_CHARACTER_ERR"));
- assertTrue(e.getMessage().contains(
- "Around ...<div id=\"div_super\" class=\"div_super\"..."));
- } catch (GadgetException e) {
- assertEquals(GadgetException.Code.HTML_PARSE_ERROR, e.getCode());
- }
+ Document doc = parser.parseDom(content);
+
+ // Returns a bare Document with error text in it.
+ Node body = doc.getElementsByTagName("body").item(0);
+
+ assertTrue(body.getTextContent().contains("INVALID_CHARACTER_ERR"));
+ assertTrue(body.getTextContent().contains(
+ "Around ...<div id=\"div_super\" class=\"div_super\"..."));
}
private void assertEmpty(Node n) {