Navigator API changes based on review (#8859)

- Simplify the default providers and move addView()/removeView() logic
to Navigator itself.
- use longest matching view name to manage view hierarchy
- minor cleanup

2 files changed, 290 insertions, 130 deletions

diff --git a/tests/server-side/com/vaadin/tests/server/navigator/ClassBasedViewProviderTest.java b/tests/server-side/com/vaadin/tests/server/navigator/ClassBasedViewProviderTest.java
index 72d595bc7d..fc0d1b60a0 100644
--- a/tests/server-side/com/vaadin/tests/server/navigator/ClassBasedViewProviderTest.java
+++ b/tests/server-side/com/vaadin/tests/server/navigator/ClassBasedViewProviderTest.java
@@ -12,8 +12,6 @@ import com.vaadin.ui.Label;
 
 public class ClassBasedViewProviderTest extends TestCase {
 
-    private ClassBasedViewProvider provider;
-
     public static class TestView extends Label implements View {
         public String parameters = null;
 
@@ -27,86 +25,68 @@ public class ClassBasedViewProviderTest extends TestCase {
 
     }
 
-    @Override
-    protected void setUp() throws Exception {
-        super.setUp();
-        provider = new ClassBasedViewProvider();
-    }
-
-    public void testAddViewWithNullName() throws Exception {
+    public void testCreateProviderWithNullName() throws Exception {
         try {
-            provider.addView(null, TestView.class);
-            fail("Should not be able to add view with null name");
+            new ClassBasedViewProvider(null, TestView.class);
+            fail("Should not be able to create view provider with null name");
         } catch (IllegalArgumentException e) {
         }
     }
 
-    public void testAddViewWithEmptyStringName() throws Exception {
-        try {
-            provider.addView("", TestView.class);
-            fail("Should not be able to add view with empty name");
-        } catch (IllegalArgumentException e) {
-        }
+    public void testCreateProviderWithEmptyStringName() throws Exception {
+        new ClassBasedViewProvider("", TestView.class);
     }
 
-    public void testAddViewNull() throws Exception {
+    public void testCreateProviderNullViewClass() throws Exception {
         try {
-            provider.addView("test", null);
-            fail("Should not be able to add null view");
+            new ClassBasedViewProvider("test", null);
+            fail("Should not be able to create view provider with null view class");
         } catch (IllegalArgumentException e) {
         }
     }
 
-    public void testAddViewSameName() throws Exception {
-        try {
-            provider.addView("test", TestView.class);
-            provider.addView("test", TestView2.class);
-            fail("Should not be able to add two views with same name");
-        } catch (IllegalArgumentException e) {
-        }
+    public void testViewNameGetter() throws Exception {
+        ClassBasedViewProvider provider1 = new ClassBasedViewProvider("",
+                TestView.class);
+        assertEquals("View name should be empty", "", provider1.getViewName());
+
+        ClassBasedViewProvider provider2 = new ClassBasedViewProvider("test",
+                TestView.class);
+        assertEquals("View name does not match", "test",
+                provider2.getViewName());
     }
 
-    public void testAddViewSameClass() throws Exception {
-        try {
-            provider.addView("test", TestView.class);
-            provider.addView("test2", TestView.class);
-            fail("Should not be able to add same view class with two different names");
-        } catch (IllegalArgumentException e) {
-        }
+    public void testViewClassGetter() throws Exception {
+        ClassBasedViewProvider provider = new ClassBasedViewProvider("test",
+                TestView.class);
+        assertEquals("Incorrect view class returned by getter", TestView.class,
+                provider.getViewClass());
     }
 
     public void testGetViewNameForNullString() throws Exception {
-        assertNull(
-                "Found view name for null view string in an empty view provider",
-                provider.getViewName((String) null));
-
-        provider.addView("test", TestView.class);
-        assertNull("Found view name for null view string",
+        ClassBasedViewProvider provider = new ClassBasedViewProvider("test",
+                TestView.class);
+        assertNull("Received view name for null view string",
                 provider.getViewName((String) null));
     }
 
-    public void testGetViewNameForNullClass() throws Exception {
-        assertNull("Found view name for null class",
-                provider.getViewName((Class<View>) null));
-    }
-
     public void testGetViewNameForEmptyString() throws Exception {
-        assertNull(
-                "Found view name for empty view string in an empty provider",
-                provider.getViewName(""));
-        provider.addView("test", TestView.class);
-        assertNull("Found view name for empty view string",
-                provider.getViewName(""));
-    }
+        ClassBasedViewProvider provider1 = new ClassBasedViewProvider("",
+                TestView.class);
+        assertEquals(
+                "Did not find view name for empty view string in a provider with empty string registered",
+                "", provider1.getViewName(""));
 
-    public void testGetViewNameForClass() throws Exception {
-        provider.addView("test", TestView.class);
-        assertEquals("No view name found for view class", "test",
-                provider.getViewName(TestView.class));
+        ClassBasedViewProvider provider2 = new ClassBasedViewProvider("test",
+                TestView.class);
+        assertNull(
+                "Found view name for empty view string when none registered",
+                provider2.getViewName(""));
     }
 
     public void testGetViewNameWithParameters() throws Exception {
-        provider.addView("test", TestView.class);
+        ClassBasedViewProvider provider = new ClassBasedViewProvider("test",
+                TestView.class);
         assertEquals("Incorrect view name found for view string", "test",
                 provider.getViewName("test"));
         assertEquals(
@@ -117,86 +97,21 @@ public class ClassBasedViewProviderTest extends TestCase {
                 "test", provider.getViewName("test/params/are/here"));
     }
 
-    public void testGetViewNameMultipleRegisteredWithParameters()
-            throws Exception {
-        provider.addView("test", TestView.class);
-        provider.addView("test2", TestView2.class);
-        assertEquals("Incorrect view name found for view string", "test",
-                provider.getViewName("test/test2/params"));
-    }
+    public void testGetView() throws Exception {
+        ClassBasedViewProvider provider = new ClassBasedViewProvider("test",
+                TestView.class);
 
-    public void testGetViewNameNestedNames() throws Exception {
-        provider.addView("test/subview", TestView2.class);
-        provider.addView("test", TestView.class);
-        assertEquals("Incorrect view name found for subview string",
-                "test/subview", provider.getViewName("test/subview"));
-        assertEquals(
-                "Incorrect view name found for subview string with empty parameters",
-                "test/subview", provider.getViewName("test/subview/"));
-        assertEquals(
-                "Incorrect view name found for subview string with parameters",
-                "test/subview", provider.getViewName("test/subview/parameters"));
-        assertEquals("Incorrect view name found for top level view string",
-                "test", provider.getViewName("test"));
-        assertEquals(
-                "Incorrect view name found for top level view string with empty parameters",
-                "test", provider.getViewName("test/"));
-        assertEquals(
-                "Incorrect view name found for top level view string with parameters starting like subview name",
-                "test", provider.getViewName("test/subviewnothere"));
-    }
-
-    public void testGetViewClass() throws Exception {
-        assertNull("View class found for empty view provider",
-                provider.getViewClass("test"));
-        provider.addView("test", TestView.class);
-        assertEquals("View class not found", TestView.class,
-                provider.getViewClass("test"));
-        assertNull("View class found for unregistered view name",
-                provider.getViewClass("test2"));
-    }
-
-    public void testGetViewSimple() throws Exception {
-        assertNull("Found view in an empty view provider",
-                provider.getViewName("test"));
-
-        provider.addView("test", TestView.class);
         View view = provider.getView("test");
         assertNotNull("Did not get view from a provider", view);
         assertEquals("Incorrect view type", TestView.class, view.getClass());
     }
 
-    public void testGetViewMultipleRegistered() throws Exception {
-        provider.addView("test", TestView.class);
-        provider.addView("test2", TestView2.class);
-        assertEquals("Incorrect view type", TestView.class,
-                provider.getView("test").getClass());
-        assertEquals("Incorrect view type", TestView2.class,
-                provider.getView("test2").getClass());
-        assertEquals("Incorrect view type", TestView.class,
-                provider.getView("test").getClass());
-    }
-
-    public void testRemoveView() throws Exception {
-        provider.addView("test", TestView.class);
-        assertNotNull("Did not get view from a provider",
-                provider.getView("test"));
-        provider.removeView("test");
-        assertNull("View class found for removed view name",
-                provider.getViewClass("test"));
-        assertNull("View name found for removed view",
-                provider.getViewName(TestView.class));
-        // cached view?
-        assertNull(
-                "Received view instance from a provider after removing view type",
-                provider.getView("test"));
-    }
+    public void testGetViewIncorrectViewName() throws Exception {
+        ClassBasedViewProvider provider = new ClassBasedViewProvider("test",
+                TestView.class);
 
-    public void testGetViewCached() throws Exception {
-        provider.addView("test", TestView.class);
-        View view1 = provider.getView("test");
-        View view2 = provider.getView("test");
-        assertSame("View instance not cached", view1, view2);
+        View view = provider.getView("test2");
+        assertNull("Got view from a provider for incorrect view name", view);
     }
 
 }
diff --git a/tests/server-side/com/vaadin/tests/server/navigator/NavigatorTest.java b/tests/server-side/com/vaadin/tests/server/navigator/NavigatorTest.java
index 49cd82c2e3..ab983b4022 100644
--- a/tests/server-side/com/vaadin/tests/server/navigator/NavigatorTest.java
+++ b/tests/server-side/com/vaadin/tests/server/navigator/NavigatorTest.java
@@ -13,17 +13,60 @@ import org.easymock.IMocksControl;
 
 import com.vaadin.navigator.FragmentManager;
 import com.vaadin.navigator.Navigator;
+import com.vaadin.navigator.Navigator.SimpleViewDisplay;
 import com.vaadin.navigator.View;
 import com.vaadin.navigator.ViewChangeListener;
 import com.vaadin.navigator.ViewChangeListener.ViewChangeEvent;
 import com.vaadin.navigator.ViewDisplay;
 import com.vaadin.navigator.ViewProvider;
+import com.vaadin.tests.server.navigator.ClassBasedViewProviderTest.TestView;
+import com.vaadin.tests.server.navigator.ClassBasedViewProviderTest.TestView2;
+import com.vaadin.ui.Root;
 
 public class NavigatorTest extends TestCase {
 
     // TODO test internal parameters (and absence of them)
     // TODO test listeners blocking navigation, multiple listeners
 
+    public static class NullDisplay implements ViewDisplay {
+        public void showView(View view) {
+            // do nothing
+        }
+    }
+
+    public static class NullFragmentManager implements FragmentManager {
+        public String getFragment() {
+            return null;
+        }
+
+        public void setFragment(String fragment) {
+            // do nothing
+        }
+    }
+
+    public static class TestDisplay implements ViewDisplay {
+        private View currentView;
+
+        public void showView(View view) {
+            currentView = view;
+        }
+
+        public View getCurrentView() {
+            return currentView;
+        }
+    }
+
+    public static class TestNavigator extends Navigator {
+        public TestNavigator() {
+            super(new NullFragmentManager(), new TestDisplay());
+        }
+
+        public View getView(String viewAndParameters) {
+            navigateTo(viewAndParameters);
+            return ((TestDisplay) getDisplay()).getCurrentView();
+        }
+    }
+
     public static class ViewChangeTestListener implements ViewChangeListener {
         private final LinkedList<ViewChangeEvent> referenceEvents = new LinkedList<ViewChangeListener.ViewChangeEvent>();
         private final LinkedList<Boolean> referenceIsCheck = new LinkedList<Boolean>();
@@ -321,4 +364,206 @@ public class NavigatorTest extends TestCase {
         }
     }
 
+    public void testDefaultDisplayType() {
+        IMocksControl control = EasyMock.createControl();
+        Root root = control.createMock(Root.class);
+
+        Navigator navigator = new Navigator(root);
+
+        assertEquals("Default display should be a SimpleViewDisplay",
+                SimpleViewDisplay.class, navigator.getDisplay().getClass());
+    }
+
+    public void testAddViewInstance() throws Exception {
+        View view = new TestView();
+
+        TestNavigator navigator = new TestNavigator();
+
+        navigator.addView("test", view);
+
+        assertEquals("Registered view instance not returned by navigator",
+                view, navigator.getView("test"));
+    }
+
+    public void testAddViewInstanceSameName() throws Exception {
+        View view1 = new TestView();
+        View view2 = new TestView2();
+
+        TestNavigator navigator = new TestNavigator();
+
+        navigator.addView("test", view1);
+        navigator.addView("test", view2);
+
+        assertEquals(
+                "Adding second view with same name should override previous view",
+                view2, navigator.getView("test"));
+    }
+
+    public void testAddViewClass() throws Exception {
+        TestNavigator navigator = new TestNavigator();
+
+        navigator.addView("test", TestView.class);
+
+        View view = navigator.getView("test");
+        assertNotNull("Received null view", view);
+        assertEquals("Received incorrect type of view", TestView.class,
+                view.getClass());
+    }
+
+    public void testAddViewClassSameName() throws Exception {
+        TestNavigator navigator = new TestNavigator();
+
+        navigator.addView("test", TestView.class);
+        navigator.addView("test", TestView2.class);
+
+        assertEquals(
+                "Adding second view class with same name should override previous view",
+                TestView2.class, navigator.getView("test").getClass());
+    }
+
+    public void testAddViewInstanceAndClassSameName() throws Exception {
+        TestNavigator navigator = new TestNavigator();
+
+        navigator.addView("test", TestView.class);
+        TestView2 view2 = new TestView2();
+        navigator.addView("test", view2);
+
+        assertEquals(
+                "Adding second view class with same name should override previous view",
+                view2, navigator.getView("test"));
+
+        navigator.addView("test", TestView.class);
+
+        assertEquals(
+                "Adding second view class with same name should override previous view",
+                TestView.class, navigator.getView("test").getClass());
+    }
+
+    public void testAddViewWithNullName() throws Exception {
+        Navigator navigator = new Navigator(new NullFragmentManager(),
+                new NullDisplay());
+
+        try {
+            navigator.addView(null, new TestView());
+            fail("addView() accepted null view name");
+        } catch (IllegalArgumentException e) {
+        }
+        try {
+            navigator.addView(null, TestView.class);
+            fail("addView() accepted null view name");
+        } catch (IllegalArgumentException e) {
+        }
+    }
+
+    public void testAddViewWithNullInstance() throws Exception {
+        Navigator navigator = new Navigator(new NullFragmentManager(),
+                new NullDisplay());
+
+        try {
+            navigator.addView("test", (View) null);
+            fail("addView() accepted null view instance");
+        } catch (IllegalArgumentException e) {
+        }
+    }
+
+    public void testAddViewWithNullClass() throws Exception {
+        Navigator navigator = new Navigator(new NullFragmentManager(),
+                new NullDisplay());
+
+        try {
+            navigator.addView("test", (Class<View>) null);
+            fail("addView() accepted null view class");
+        } catch (IllegalArgumentException e) {
+        }
+    }
+
+    public void testRemoveViewInstance() throws Exception {
+        View view = new TestView();
+
+        TestNavigator navigator = new TestNavigator();
+
+        navigator.addView("test", view);
+        navigator.removeView("test");
+
+        assertNull("View not removed", navigator.getView("test"));
+    }
+
+    public void testRemoveViewInstanceNothingElse() throws Exception {
+        View view = new TestView();
+        View view2 = new TestView2();
+
+        TestNavigator navigator = new TestNavigator();
+
+        navigator.addView("test", view);
+        navigator.addView("test2", view2);
+        navigator.removeView("test");
+
+        assertEquals("Removed extra views", view2, navigator.getView("test2"));
+    }
+
+    public void testRemoveViewClass() throws Exception {
+        TestNavigator navigator = new TestNavigator();
+
+        navigator.addView("test", TestView.class);
+        navigator.removeView("test");
+
+        assertNull("View not removed", navigator.getView("test"));
+    }
+
+    public void testRemoveViewClassNothingElse() throws Exception {
+        TestNavigator navigator = new TestNavigator();
+
+        navigator.addView("test", TestView.class);
+        navigator.addView("test2", TestView2.class);
+        navigator.removeView("test");
+
+        assertEquals("Removed extra views", TestView2.class,
+                navigator.getView("test2").getClass());
+    }
+
+    public void testGetViewNestedNames() throws Exception {
+        TestNavigator navigator = new TestNavigator();
+
+        navigator.addView("test/subview", TestView2.class);
+        navigator.addView("test", TestView.class);
+
+        assertEquals("Incorrect view name found for subview string",
+                TestView2.class, navigator.getView("test/subview").getClass());
+        assertEquals(
+                "Incorrect view name found for subview string with empty parameters",
+                TestView2.class, navigator.getView("test/subview/").getClass());
+        assertEquals(
+                "Incorrect view name found for subview string with parameters",
+                TestView2.class, navigator.getView("test/subview/parameters")
+                        .getClass());
+        assertEquals("Incorrect view name found for top level view string",
+                TestView.class, navigator.getView("test").getClass());
+        assertEquals(
+                "Incorrect view name found for top level view string with empty parameters",
+                TestView.class, navigator.getView("test/").getClass());
+        assertEquals(
+                "Incorrect view name found for top level view string with parameters starting like subview name",
+                TestView.class, navigator.getView("test/subviewnothere")
+                        .getClass());
+    }
+
+    public void testGetViewLongestPrefixOrder() throws Exception {
+        TestNavigator navigator = new TestNavigator();
+
+        navigator.addView("test/subview", TestView2.class);
+        navigator.addView("test", TestView.class);
+
+        assertEquals("Incorrect view name found", TestView.class, navigator
+                .getView("test").getClass());
+
+        // other order
+
+        TestNavigator navigator2 = new TestNavigator();
+
+        navigator2.addView("test", TestView.class);
+        navigator2.addView("test/subview", TestView2.class);
+
+        assertEquals("Incorrect view name found", TestView.class, navigator2
+                .getView("test").getClass());
+    }
 }