Break cycles created by Symlinks.

Move this logic to symlinks, and not in the page crawler. This implies that things like searches work over symlinks as well.

Fixes unclebob#488.

src/fitnesse/wiki/PageCrawlerImpl.java

@@ -126,8 +126,6 @@ public void traverse(TraversalListener<? super WikiPage> listener) {
   }
 
   private void traverse(WikiPage page, TraversalListener<? super WikiPage> listener) {
-    if (page.getClass() == SymbolicPage.class)
-      return;
     listener.process(page);
     for (WikiPage wikiPage : page.getChildren()) {
       traverse(wikiPage, listener);

src/fitnesse/wiki/SymbolicPage.java

@@ -12,13 +12,13 @@
 public class SymbolicPage extends BaseWikitextPage {
 
   public static final String PROPERTY_NAME = "SymbolicLinks";
+  public static final String SHORT_CIRCUIT_BREAK_MESSAGE = "Short circuit! This page references %s, which is already one of the parent pages of this page.";
 
   private final WikiPage realPage;
 
   public SymbolicPage(String name, WikiPage realPage, WikiPage parent) {
     super(name, parent);
     this.realPage = realPage;
-    // Perform a cyclic dependency check
   }
 
   public WikiPage getRealPage() {
@@ -43,9 +43,8 @@ public boolean hasChildPage(String name) {
   public WikiPage getChildPage(String name) {
     WikiPage childPage = realPage.getChildPage(name);
     if (childPage != null) {
-      childPage = new SymbolicPage(name, childPage, this);
+      childPage = createChildPage(childPage);
     }
-
     return childPage;
   }
 
@@ -58,13 +57,30 @@ public void removeChildPage(String name) {
   public List<WikiPage> getChildren() {
     List<WikiPage> children = realPage.getChildren();
     List<WikiPage> symChildren = new LinkedList<WikiPage>();
-    //TODO: -AcD- we need a better cyclic infinite recursion algorithm here.
     for (WikiPage child : children) {
-      symChildren.add(new SymbolicPage(child.getName(), child, this));
+      symChildren.add(createChildPage(child));
     }
     return symChildren;
   }
 
+  private WikiPage createChildPage(WikiPage child) {
+    WikiPage cyclicReference = findCyclicReference(child);
+    if (cyclicReference != null) {
+      return new WikiPageDummy(child.getName(), String.format(SHORT_CIRCUIT_BREAK_MESSAGE, cyclicReference.getPageCrawler().getFullPath().toString()), this);
+    } else {
+      return new SymbolicPage(child.getName(), child, this);
+    }
+  }
+
+  private WikiPage findCyclicReference(WikiPage childPage) {
+    for (WikiPage parentPage = getParent(); !parentPage.isRoot(); parentPage = parentPage.getParent()) {
+      if (childPage.equals(parentPage)) {
+        return parentPage;
+      }
+    }
+    return null;
+  }
+
   @Override
   public PageData getData() {
     return realPage.getData();
@@ -118,6 +134,17 @@ public Symbol getSyntaxTree() {
     return Symbol.emptySymbol;
   }
 
+  @Override
+  @SuppressWarnings("EqualsWhichDoesntCheckParameterClass")
+  public boolean equals(Object other) {
+    return realPage.equals(other);
+  }
+
+  @Override
+  public int hashCode() {
+    return realPage.hashCode();
+  }
+
   public static boolean containsWikitext(WikiPage wikiPage) {
     if (wikiPage instanceof SymbolicPage) {
       return containsWikitext(((SymbolicPage) wikiPage).realPage);

src/fitnesse/wiki/WikiPageDummy.java

@@ -42,7 +42,7 @@ public VersionInfo commit(PageData data) {
 
   @Override
   public List<WikiPage> getChildren() {
-    return new ArrayList<WikiPage>();
+    return new ArrayList<>();
   }
 
   @Override
@@ -52,7 +52,7 @@ public WikiPage getVersion(String versionName) {
 
   @Override
   public String getHtml() {
-    return "";
+    return String.format("<em>%s</em>", pageData.getContent());
   }
 
   @Override

test/fitnesse/wiki/PageCrawlerTest.java

@@ -156,21 +156,30 @@ public void testTraversal() throws Exception {
     assertTrue(traversedPages.contains("ChildOne"));
   }
 
-  @Override
-  public void process(WikiPage page) {
-    traversedPages.add(page.getName());
-  }
-
   @Test
-  public void testdoesntTraverseSymbolicPages() throws Exception {
+  public void doesTraverseSymbolicPages() throws Exception {
     PageData data = page1.getData();
-    data.getProperties().set(SymbolicPage.PROPERTY_NAME).set("SymLink", "PageTwo");
+    data.getProperties().set(SymbolicPage.PROPERTY_NAME).set("SymLink", page2.getName());
     page1.commit(data);
 
     crawler.traverse(this);
-    assertEquals(6, traversedPages.size());
+    assertEquals(7, traversedPages.size());
+
+    assertTrue(traversedPages.contains("SymLink"));
+  }
+
+  @Test
+  public void doesNotTraverseCyclicPageReferences() {
+    PageData data = child1.getData();
+    data.getProperties().set(SymbolicPage.PROPERTY_NAME).set("SymLink", "." + page1.getName());
+    child1.commit(data);
+
+    crawler = new PageCrawlerImpl(page1);
+    crawler.traverse(this);
 
-    assertFalse(traversedPages.contains("SymLink"));
+    assertEquals(traversedPages.toString(), 5, traversedPages.size());
+
+    assertTrue(traversedPages.contains("SymLink"));
   }
 
   @Test
@@ -190,4 +199,9 @@ public void process(WikiPage page) {
     assertTrue(uncles.contains(brotherPage));
 
   }
+
+  @Override
+  public void process(WikiPage page) {
+    traversedPages.add(page.getName());
+  }
 }

test/fitnesse/wiki/SymbolicPageTest.java

@@ -119,28 +119,39 @@ public void nonWikitextPageShouldReadParentVariable() {
 
   @Test
   public void testCyclicSymbolicLinks() throws Exception {
+    createCycle();
+    PageCrawler pageCrawler = root.getPageCrawler();
+
+    WikiPage deepPage = pageCrawler.getPage(PathParser.parse(pageOnePath + ".SymOne.SymTwo.SymOne.SymTwo.SymOne"));
+    assertNull(deepPage);
+
+  }
+
+  @Test
+  public void cyclicSymbolicLinkDisplaysAMessage() {
+    createCycle();
+    PageCrawler pageCrawler = root.getPageCrawler();
+    WikiPage deepPage = pageCrawler.getPage(PathParser.parse(pageTwoPath + ".SymTwo.SymOne"));
+    List<WikiPage> children = deepPage.getChildren();
+    assertEquals(0, children.size());
+    assertEquals("<em>Short circuit! This page references PageTwo, which is already one of the parent pages of this page.</em>", deepPage.getHtml());
+  }
+
+  private void createCycle() {
     PageData data = pageOne.getData();
     data.getProperties().set(SymbolicPage.PROPERTY_NAME).set("SymOne", pageTwoPath);
     pageOne.commit(data);
 
     data = pageTwo.getData();
     data.getProperties().set(SymbolicPage.PROPERTY_NAME).set("SymTwo", pageOnePath);
     pageTwo.commit(data);
-    PageCrawler pageCrawler = root.getPageCrawler();
-    WikiPage deepPage = pageCrawler.getPage(PathParser.parse(pageOnePath + ".SymOne.SymTwo.SymOne.SymTwo.SymOne"));
-    List<?> children = deepPage.getChildren();
-    assertEquals(1, children.size());
-
-    deepPage = pageCrawler.getPage(PathParser.parse(pageTwoPath + ".SymTwo.SymOne.SymTwo.SymOne.SymTwo"));
-    children = deepPage.getChildren();
-    assertEquals(1, children.size());
   }
 
   @Test
   public void nestedSymbolicLinksShouldKeepTheRightPath() {
     String pageThreePath = "PageThree";
     String pageThreeContent = "page three";
-    WikiPage pageThree = WikiPageUtil.addPage(root, PathParser.parse(pageThreePath), pageThreeContent);
+    WikiPageUtil.addPage(root, PathParser.parse(pageThreePath), pageThreeContent);
 
     PageData data = pageOne.getData();
     data.getProperties().set(SymbolicPage.PROPERTY_NAME).set("SymOne", pageTwoPath);
@@ -158,7 +169,7 @@ public void nestedSymbolicLinksShouldKeepTheRightPath() {
 
   @Test
   public void testSymbolicPageUsingExternalDirectory() throws Exception {
-    CreateExternalRoot();
+    createExternalRoot();
 
     assertEquals(2, symPage.getChildren().size());
 
@@ -175,7 +186,7 @@ public void testSymbolicPageUsingExternalDirectory() throws Exception {
     assertEquals("external child", symChild.getData().getContent());
   }
 
-  private void CreateExternalRoot() throws Exception {
+  private void createExternalRoot() throws Exception {
     FileUtil.createDir("testDir");
     FileUtil.createDir("testDir/ExternalRoot");
     externalRoot = new FileSystemPageFactory().makePage(new File("testDir/ExternalRoot"), "ExternalRoot", null, new SystemVariableSource());
@@ -188,7 +199,7 @@ private void CreateExternalRoot() throws Exception {
 
   @Test
   public void testCommittingToExternalRoot() throws Exception {
-    CreateExternalRoot();
+    createExternalRoot();
 
     commitNewContent(symPage);