Curator 94

curator-x-discovery/src/main/java/org/apache/curator/x/discovery/ServiceType.java

@@ -18,9 +18,39 @@
  */
 package org.apache.curator.x.discovery;
 
+/**
+ * The type of the service registration.
+ *
+ * STATIC and PERMANENT registrations are created when the x-discovery-server rest service registers another service.
+ * Where as DYNAMIC registrations are typically used for everything else
+ * (including the definition of the x-discovery-server rest service itself).
+ */
 public enum ServiceType
 {
+    /**
+     * DYNAMIC registrations (default) are tied to the lifecycle of the creating process.
+     *
+     * ServiceDiscoveryImpl.internalRegisterService() maps DYNAMIC registrations to EPHEMERAL nodes as a result they
+     * don't need to be manually cleaned up.
+     */
     DYNAMIC,
+
+    /**
+     * STATIC registrations require the caller to regularly re-register the service before a timeout expires.
+     *
+     * The timeout is not defined in the curator code (the caller must define the timeout).
+     *
+     * However InstanceCleanup.checkService implements the timeout process to expire them after the timeout.
+     * Even though STATIC registrations are mapped to PERSISTENT zookeeper nodes, they are also cleaned up
+     * during a clean shutdown of the x-discovery-server rest service ServiceDiscoveryImpl.close().
+     */
     STATIC,
+
+    /**
+     * PERMANENT registrations are not tied to the existence of any particular process.
+     *
+     * They can be (un)registered through the x-discovery-server rest service, however the process immediately discards
+     * all knowledge of the registration ServiceDiscoveryImpl.registerService(...)
+     */
     PERMANENT
 }

curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/ServiceDiscoveryImpl.java

@@ -146,7 +146,10 @@ public void close() throws IOException
     @Override
     public void registerService(ServiceInstance<T> service) throws Exception
     {
-        services.put(service.getId(), service);
+        if (service.getServiceType() != ServiceType.PERMANENT)
+        {
+            services.put(service.getId(), service);
+        }
         internalRegisterService(service);
     }
 

curator-x-discovery/src/test/java/org/apache/curator/x/discovery/TestServiceDiscovery.java

@@ -273,4 +273,97 @@ public void         testBasic() throws Exception
             }
         }
     }
+
+    @Test
+    public void testCloseBehavior() throws Exception
+    {
+
+        TestingServer server = new TestingServer();
+        try
+        {
+            List<Closeable> closeables = Lists.newArrayList();
+
+            ServiceInstance<String> testReg = ServiceInstance.<String>builder().payload("thing").name("test").port(10064).build();
+
+            ServiceInstance<String> dynamicReg = ServiceInstance.<String>builder().payload("dynamic").name("dynamic").port(1111).serviceType(ServiceType.DYNAMIC).build();
+            ServiceInstance<String> staticReg = ServiceInstance.<String>builder().payload("static").name("static").port(1112).serviceType(ServiceType.STATIC).build();
+            ServiceInstance<String> permanentReg = ServiceInstance.<String>builder().payload("permanent").name("permanent").port(1113).serviceType(ServiceType.PERMANENT).build();
+
+            try
+            {
+                CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+                closeables.add(client);
+                client.start();
+
+                final ServiceDiscoveryBuilder<String> builder = ServiceDiscoveryBuilder.builder(String.class).basePath("/test").client(client).thisInstance(testReg);
+                ServiceDiscovery<String> discovery = builder.build();
+                closeables.add(discovery);
+                discovery.start();
+
+                Assert.assertEquals(discovery.queryForNames(), Arrays.asList("test"));
+
+                List<ServiceInstance<String>> list = Lists.newArrayList();
+                list.add(testReg);
+                Assert.assertEquals(discovery.queryForInstances("test"), list);
+
+                discovery.registerService(dynamicReg);
+                discovery.registerService(staticReg);
+                discovery.registerService(permanentReg);
+            }
+            finally
+            {
+                Collections.reverse(closeables);
+                for (Closeable c : closeables)
+                {
+                    CloseableUtils.closeQuietly(c);
+                }
+            }
+
+            closeables.clear();
+
+            try
+            {
+                CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+                closeables.add(client);
+                client.start();
+
+                ServiceDiscovery<String> discovery = ServiceDiscoveryBuilder.builder(String.class).basePath("/test").client(client).thisInstance(testReg).build();
+                closeables.add(discovery);
+                discovery.start();
+
+                Assert.assertEquals(Sets.newHashSet(discovery.queryForNames()), Sets.newHashSet("test", "dynamic", "static", "permanent"));
+
+                List<ServiceInstance<String>> testRegistrations = Lists.newArrayList();
+                testRegistrations.add(testReg);
+
+                final List<ServiceInstance<String>> emptyList = Lists.newArrayList();
+
+
+                List<ServiceInstance<String>> permanentRegistrations = Lists.newArrayList();
+                permanentRegistrations.add(permanentReg);
+
+                Assert.assertEquals(discovery.queryForInstances("test"), testRegistrations);
+                Assert.assertEquals(discovery.queryForInstances("dynamic"), emptyList);
+                Assert.assertEquals(discovery.queryForInstances("static"), emptyList);
+                Assert.assertEquals(discovery.queryForInstances("permanent"), permanentRegistrations);
+
+                // Ensure Unregister works too.
+                discovery.unregisterService(permanentReg);
+                Assert.assertEquals(discovery.queryForInstances("permanent"), emptyList);
+            }
+            finally
+            {
+                Collections.reverse(closeables);
+                for (Closeable c : closeables)
+                {
+                    CloseableUtils.closeQuietly(c);
+                }
+            }
+
+
+        } finally
+        {
+            CloseableUtils.closeQuietly(server);
+        }
+    }
 }