Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Rolled AbstractDiscoveryLifecycle back and deprecated.
Created AbstractAutoServiceRegistration with new methods and reference to ServiceRegistry.

It extends AbstractDiscoveryLifecycle.
  • Loading branch information
spencergibb committed Nov 4, 2016
commit 8d10450ee15c92eb72694ffd87782844fd5c7b0b
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,14 @@
import org.springframework.core.env.Environment;

/**
* Lifecycle methods that may be useful and common to {@link ServiceRegistry} implementations.
* Lifecycle methods that may be useful and common to various DiscoveryClient implementations.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it still various? or is registration (the R type) intended to work only with one sort of client now? I'd probably doc the type param regardless of whether it stays a marker or not.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

R will be specific to the impl (though it won't extend Registration).

*
* TODO: document the lifecycle
*
* @param <R> registration type passed to the {@link ServiceRegistry}.
* @deprecated use {@link org.springframework.cloud.client.serviceregistry.AbstractAutoServiceRegistration} instead. This class will be removed in the next release train.
*
* @author Spencer Gibb
*/
//TODO: rename to AbstractServiceRegistryLifecycle or AbstractAutoServiceRegistration?
public abstract class AbstractDiscoveryLifecycle<R> implements DiscoveryLifecycle,
@Deprecated
public abstract class AbstractDiscoveryLifecycle implements DiscoveryLifecycle,
ApplicationContextAware, ApplicationListener<EmbeddedServletContainerInitializedEvent> {

private static final Log logger = LogFactory.getLog(AbstractDiscoveryLifecycle.class);
Expand All @@ -59,12 +57,6 @@ public abstract class AbstractDiscoveryLifecycle<R> implements DiscoveryLifecycl

private AtomicInteger port = new AtomicInteger(0);

private ServiceRegistry<R> serviceRegistry;

protected AbstractDiscoveryLifecycle(ServiceRegistry<R> serviceRegistry) {
this.serviceRegistry = serviceRegistry;
}

protected ApplicationContext getContext() {
return context;
}
Expand Down Expand Up @@ -140,40 +132,27 @@ protected boolean shouldRegisterManagement() {
*/
protected abstract Object getConfiguration();

protected abstract R getRegistration();

protected abstract R getManagementRegistration();

protected ServiceRegistry<R> getServiceRegistry() {
return this.serviceRegistry;
}

/**
* Register the local service with the {@link ServiceRegistry}
* Register the local service with the DiscoveryClient

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this doc need to be changed now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, DiscoveryClient was never right, since it wasn't used.

*/
protected void register() {
this.serviceRegistry.register(getRegistration());
}
protected abstract void register();

/**
* Register the local management service with the {@link ServiceRegistry}
* Register the local management service with the DiscoveryClient
*/
protected void registerManagement() {
this.serviceRegistry.register(getManagementRegistration());
}

/**
* De-register the local service with the {@link ServiceRegistry}
* De-register the local service with the DiscoveryClient

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

*/
protected void deregister() {
this.serviceRegistry.deregister(getRegistration());
}
protected abstract void deregister();

/**
* De-register the local management service with the {@link ServiceRegistry}
* De-register the local management service with the DiscoveryClient

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

*/
protected void deregisterManagement() {
this.serviceRegistry.deregister(getManagementRegistration());
}

/**
Expand Down Expand Up @@ -231,6 +210,10 @@ public boolean isRunning() {
return this.running.get();
}

protected AtomicBoolean getRunning() {
return running;
}

@Override
public int getOrder() {
return this.order;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package org.springframework.cloud.client.serviceregistry;

import org.springframework.cloud.client.discovery.AbstractDiscoveryLifecycle;

/**
* Lifecycle methods that may be useful and common to {@link ServiceRegistry} implementations.
*
* TODO: document the lifecycle
*
* @param <R> registration type passed to the {@link ServiceRegistry}.
*
* @author Spencer Gibb
*/
@SuppressWarnings("deprecation")
public abstract class AbstractAutoServiceRegistration<R> extends AbstractDiscoveryLifecycle {

private ServiceRegistry<R> serviceRegistry;

protected AbstractAutoServiceRegistration(ServiceRegistry<R> serviceRegistry) {
this.serviceRegistry = serviceRegistry;
}

protected ServiceRegistry<R> getServiceRegistry() {
return this.serviceRegistry;
}

protected abstract R getRegistration();

protected abstract R getManagementRegistration();

/**
* Register the local service with the {@link ServiceRegistry}
*/
@Override
protected void register() {
this.serviceRegistry.register(getRegistration());
}

/**
* Register the local management service with the {@link ServiceRegistry}
*/
@Override
protected void registerManagement() {
this.serviceRegistry.register(getManagementRegistration());
}

/**
* De-register the local service with the {@link ServiceRegistry}
*/
@Override
protected void deregister() {
this.serviceRegistry.deregister(getRegistration());
}

/**
* De-register the local management service with the {@link ServiceRegistry}
*/
@Override
protected void deregisterManagement() {
this.serviceRegistry.deregister(getManagementRegistration());
}

@Override
public void stop() {
if (this.getRunning().compareAndSet(true, false) && isEnabled()) {
deregister();
if (shouldRegisterManagement()) {
deregisterManagement();
}
this.serviceRegistry.close();
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,7 @@ public interface ServiceRegistry<R> {
void register(R registration);

void deregister(R registration);

void close();

}
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
package org.springframework.cloud.client.discovery;

import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.*;

import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -14,7 +9,6 @@
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.test.SpringApplicationConfiguration;
import org.springframework.boot.test.WebIntegrationTest;
import org.springframework.cloud.client.serviceregistry.ServiceRegistry;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
Expand Down Expand Up @@ -42,9 +36,7 @@ public void portsWork() {
assertNotEquals("Lifecycle port is management port", managementPort, lifecycle.getPort().get());
assertEquals("Lifecycle port is wrong", port, lifecycle.getPort().get());
assertTrue("Lifecycle not running", lifecycle.isRunning());
assertThat("ServiceRegistry is wrong type", lifecycle.getServiceRegistry(), is(instanceOf(TestServiceRegistry.class)));
TestServiceRegistry serviceRegistry = (TestServiceRegistry) lifecycle.getServiceRegistry();
assertTrue("Lifecycle not registered", serviceRegistry.isRegistered());
assertTrue("Lifecycle not registered", lifecycle.isRegistered());
assertEquals("Lifecycle appName is wrong", "application", lifecycle.getAppName());
}

Expand All @@ -57,39 +49,11 @@ public TestDiscoveryLifecycle testDiscoveryLifecycle() {
}
}

public static class TestRegistration {
}

public static class TestServiceRegistry implements ServiceRegistry<TestRegistration> {
public static class TestDiscoveryLifecycle extends AbstractDiscoveryLifecycle {
private int port = 0;
private boolean registered = false;
private boolean deregistered = false;

@Override
public void register(TestRegistration registration) {
this.registered = true;
}

@Override
public void deregister(TestRegistration registration) {
this.deregistered = true;
}

public boolean isRegistered() {
return registered;
}

public boolean isDeregistered() {
return deregistered;
}
}

public static class TestDiscoveryLifecycle extends AbstractDiscoveryLifecycle<TestRegistration> {
private int port = 0;

protected TestDiscoveryLifecycle() {
super(new TestServiceRegistry());
}

@Override
protected int getConfiguredPort() {
return port;
Expand All @@ -101,24 +65,31 @@ protected void setConfiguredPort(int port) {
}

@Override
protected TestRegistration getRegistration() {
return null;
protected Object getConfiguration() {
return this;
}

@Override
protected TestRegistration getManagementRegistration() {
return null;
protected void register() {
this.registered = true;
}

@Override
protected Object getConfiguration() {
return null;
protected void deregister() {
this.deregistered = true;
}

@Override
protected boolean isEnabled() {
return true;
}

public boolean isRegistered() {
return registered;
}

public boolean isDeregistered() {
return deregistered;
}
}
}
Loading