You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by jo...@gmail.com on 2010/04/14 04:16:29 UTC

Add change count to MutableContent (issue867049)

Reviewers: shindig.remailer_gmail.com,

Description:
Pretty simple, but I wanted to send this out for others' awareness.

In our installation we'd like to profile the number of times a given
rewriter is actually used, ie. manipulates content. This CL implements a
cheap primitive - numChanges - that allows RewriterRegistries et al to
be implemented that do this. It's acknowledged that for DOM rewriters
the accuracy of this "count" (which in practice will likely always act
as a boolean ie. beforeCount = getcount(); rewrite(); wasRewritten =
(beforeCount != getcount()) depends on the rewriter only calling
mc.documentChanged() when this is so.

Please review this at http://codereview.appspot.com/867049/show

Affected files:
    
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/MutableContent.java
    
java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/MutableContentTest.java


Index:  
java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/MutableContentTest.java
===================================================================
---  
java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/MutableContentTest.java	 
(revision 933826)
+++  
java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/MutableContentTest.java	 
(working copy)
@@ -54,14 +54,19 @@

      assertSame(content, mhc.getContent());
      assertSame(document, mhc.getDocument());
+    assertEquals(0, mhc.getNumChanges());
    }

    @Test
    public void modifyContentReflectedInTree() throws Exception {
+    assertEquals(0, mhc.getNumChanges());
      mhc.setContent("NEW CONTENT");
+    assertEquals(1, mhc.getNumChanges());
      Document document = mhc.getDocument();
      assertEquals(1, document.getChildNodes().getLength());
      assertEquals("NEW CONTENT",  
document.getChildNodes().item(0).getTextContent());
+    mhc.documentChanged();
+    assertEquals(2, mhc.getNumChanges());
    }

    @Test
@@ -70,12 +75,15 @@

      // First child should be text node per other tests. Modify it.
      document.getFirstChild().getFirstChild().setTextContent("FOO CONTENT");
+    assertEquals(0, mhc.getNumChanges());
      MutableContent.notifyEdit(document);
+    assertEquals(1, mhc.getNumChanges());
      assertTrue(mhc.getContent().contains("FOO CONTENT"));

      // Do it again
      document.getFirstChild().getFirstChild().setTextContent("BAR CONTENT");
      MutableContent.notifyEdit(document);
+    assertEquals(2, mhc.getNumChanges());
      assertTrue(mhc.getContent().contains("BAR CONTENT"));

      // GadgetHtmlNode hasn't changed because string hasn't changed
Index:  
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/MutableContent.java
===================================================================
---  
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/MutableContent.java	 
(revision 933826)
+++  
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/MutableContent.java	 
(working copy)
@@ -35,6 +35,7 @@
    private String content;
    private HttpResponse contentSource;
    private Document document;
+  private int numChanges;
    private final GadgetHtmlParser contentParser;
    private final Map<String, Object> pipelinedData;

@@ -53,7 +54,8 @@
    public MutableContent(GadgetHtmlParser contentParser, String content) {
      this.contentParser = contentParser;
      this.content = content;
-    pipelinedData = Maps.newHashMap();
+    this.numChanges = 0;
+    this.pipelinedData = Maps.newHashMap();
    }

    /**
@@ -63,7 +65,7 @@
    public MutableContent(GadgetHtmlParser contentParser, HttpResponse  
contentSource) {
      this.contentParser = contentParser;
      this.contentSource = contentSource;
-    pipelinedData = Maps.newHashMap();
+    this.pipelinedData = Maps.newHashMap();
    }


@@ -101,6 +103,7 @@
        content = newContent;
        document = null;
        contentSource = null;
+      numChanges++;
      }
    }

@@ -113,6 +116,7 @@
      if (document != null) {
        content = null;
        contentSource = null;
+      numChanges++;
      }
    }

@@ -139,6 +143,10 @@
      }
      return document;
    }
+
+  public int getNumChanges() {
+    return numChanges;
+  }

    /**
     * True if current state has a parsed document. Allows rewriters to  
switch mode based on