[ https://issues.apache.org/jira/browse/CURATOR-457?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Aleksander Melnichnikov updated CURATOR-457: -------------------------------------------- Description: Right now paths for services in service discovery hardcoded to pattern: */\{basePath} /\{serviceName}/\{instanceId}.* And that's impossible to configure paths this way: */\{basePathPart1} /\{serviceName}/\{basePathPart2}/\{instanceId}* For example: If basePath = */base1/base2/base3*, serviceName = *serviceName*, instanceId - *serviceId* then path will be */**base1**/**base2**/**base3**/**serviceName**/**serviceId*, that can't be configured like this: */**base1**/**base2/serviceName**/**base3**/**serviceId.* Because of strong encapsulation in ServiceDiscoveryImpl class (package private method modifiers) {code:java} @VisibleForTesting String pathForName(String name) { return ZKPaths.makePath(basePath, name); } @VisibleForTesting String pathForInstance(String name, String id) { return ZKPaths.makePath(pathForName(name), id); } {code} So, can we rethink visibility modifiers or add some extendable entity which would be responsible for constructing paths? For example: Create interface {code:java} /** * A constructor which constructs path to services for service discovering */ public interface DiscoveryPathConstructor { /** * Return the path where all service names registered * @return basePath */ String getBasePath(); /** * Return the path where all instances of service registered * @param name service name * @return path to service instances */ String getPathForInstances(String name); /** * Return the path where concrete instance registered * @param name service name * @param id concrete instance id * @return path to concrete instance */ String getPathForInstance(String name, String id); } {code} And make a constructor and field in ServiceDiscoveryImpl: {code:java} private final DiscoveryPathConstructor pathConstructor; /** * @param client the client * @param pathConstructor constructor for instances path * @param serializer serializer for instances (e.g. {@link JsonInstanceSerializer}) * @param thisInstance instance that represents the service that is running. The instance will get auto-registered * @param watchInstances if true, watches for changes to locally registered instances */ public ServiceDiscoveryImpl(CuratorFramework client, DiscoveryPathConstructor pathConstructor, InstanceSerializer<T> serializer, ServiceInstance<T> thisInstance, boolean watchInstances) { this.watchInstances = watchInstances; this.client = Preconditions.checkNotNull(client, "client cannot be null"); this.serializer = Preconditions.checkNotNull(serializer, "serializer cannot be null"); this.pathConstructor = Preconditions.checkNotNull(pathConstructor, "pathConstructor cannot be null"); if ( thisInstance != null ) { Entry<T> entry = new Entry<T>(thisInstance); entry.cache = makeNodeCache(thisInstance); services.put(thisInstance.getId(), entry); } } {code} You can view commit in my fork of framework, if that's ok - I can make a pull request. https://github.com/Me1kaa/curator/commit/6e6a34db7d71a84e3a53d284fc94e1fe2e3c10d8 was: Right now paths for services in service discovery hardcoded to pattern: */\{basePath} /\{serviceName}/\{instanceId}.* And that's impossible to configure paths this way: */\{basePathPart1} /\{serviceName}/{basePathPart2}/\{instanceId}** For example: If basePath = */base1/base2/base3*, serviceName = *serviceName*, instanceId - *serviceId* then path will be */**base1**/**base2**/**base3**/**serviceName**/**serviceId*, that can't be configured like this: */**base1**/**base2/serviceName**/**base3**/**serviceId.* Because of strong encapsulation in ServiceDiscoveryImpl class (package private method modifiers) {code:java} @VisibleForTesting String pathForName(String name) { return ZKPaths.makePath(basePath, name); } @VisibleForTesting String pathForInstance(String name, String id) { return ZKPaths.makePath(pathForName(name), id); } {code} So, can we rethink visibility modifiers or add some extendable entity which would be responsible for constructing paths? For example: Create interface {code:java} /** * A constructor which constructs path to services for service discovering */ public interface DiscoveryPathConstructor { /** * Return the path where all service names registered * @return basePath */ String getBasePath(); /** * Return the path where all instances of service registered * @param name service name * @return path to service instances */ String getPathForInstances(String name); /** * Return the path where concrete instance registered * @param name service name * @param id concrete instance id * @return path to concrete instance */ String getPathForInstance(String name, String id); } {code} And make a constructor and field in ServiceDiscoveryImpl: {code:java} private final DiscoveryPathConstructor pathConstructor; /** * @param client the client * @param pathConstructor constructor for instances path * @param serializer serializer for instances (e.g. {@link JsonInstanceSerializer}) * @param thisInstance instance that represents the service that is running. The instance will get auto-registered * @param watchInstances if true, watches for changes to locally registered instances */ public ServiceDiscoveryImpl(CuratorFramework client, DiscoveryPathConstructor pathConstructor, InstanceSerializer<T> serializer, ServiceInstance<T> thisInstance, boolean watchInstances) { this.watchInstances = watchInstances; this.client = Preconditions.checkNotNull(client, "client cannot be null"); this.serializer = Preconditions.checkNotNull(serializer, "serializer cannot be null"); this.pathConstructor = Preconditions.checkNotNull(pathConstructor, "pathConstructor cannot be null"); if ( thisInstance != null ) { Entry<T> entry = new Entry<T>(thisInstance); entry.cache = makeNodeCache(thisInstance); services.put(thisInstance.getId(), entry); } } {code} You can view commit in my fork of framework, if that's ok - I can make a pull request. https://github.com/Me1kaa/curator/commit/6e6a34db7d71a84e3a53d284fc94e1fe2e3c10d8 > Configuring service discovery paths > ----------------------------------- > > Key: CURATOR-457 > URL: https://issues.apache.org/jira/browse/CURATOR-457 > Project: Apache Curator > Issue Type: Improvement > Reporter: Aleksander Melnichnikov > Assignee: Jordan Zimmerman > Priority: Minor > > Right now paths for services in service discovery hardcoded to pattern: > */\{basePath} /\{serviceName}/\{instanceId}.* And that's impossible to > configure paths this way: */\{basePathPart1} > /\{serviceName}/\{basePathPart2}/\{instanceId}* > For example: > If basePath = */base1/base2/base3*, serviceName = *serviceName*, instanceId > - *serviceId* > then path will be > */**base1**/**base2**/**base3**/**serviceName**/**serviceId*, that can't be > configured like this: > */**base1**/**base2/serviceName**/**base3**/**serviceId.* > Because of strong encapsulation in ServiceDiscoveryImpl class (package > private method modifiers) > {code:java} > @VisibleForTesting > String pathForName(String name) > { > return ZKPaths.makePath(basePath, name); > } > @VisibleForTesting > String pathForInstance(String name, String id) > { > return ZKPaths.makePath(pathForName(name), id); > } > {code} > So, can we rethink visibility modifiers or add some extendable entity which > would be responsible for constructing paths? > For example: > Create interface > {code:java} > /** > * A constructor which constructs path to services for service discovering > */ > public interface DiscoveryPathConstructor > { > /** > * Return the path where all service names registered > * @return basePath > */ > String getBasePath(); > /** > * Return the path where all instances of service registered > * @param name service name > * @return path to service instances > */ > String getPathForInstances(String name); > /** > * Return the path where concrete instance registered > * @param name service name > * @param id concrete instance id > * @return path to concrete instance > */ > String getPathForInstance(String name, String id); > } > {code} > And make a constructor and field in ServiceDiscoveryImpl: > {code:java} > private final DiscoveryPathConstructor pathConstructor; > /** > * @param client the client > * @param pathConstructor constructor for instances path > * @param serializer serializer for instances (e.g. {@link > JsonInstanceSerializer}) > * @param thisInstance instance that represents the service that is > running. The instance will get auto-registered > * @param watchInstances if true, watches for changes to locally > registered instances > */ > public ServiceDiscoveryImpl(CuratorFramework client, > DiscoveryPathConstructor pathConstructor, InstanceSerializer<T> serializer, > ServiceInstance<T> thisInstance, boolean > watchInstances) > { > this.watchInstances = watchInstances; > this.client = Preconditions.checkNotNull(client, "client cannot be > null"); > this.serializer = Preconditions.checkNotNull(serializer, "serializer > cannot be null"); > this.pathConstructor = Preconditions.checkNotNull(pathConstructor, > "pathConstructor cannot be null"); > if ( thisInstance != null ) > { > Entry<T> entry = new Entry<T>(thisInstance); > entry.cache = makeNodeCache(thisInstance); > services.put(thisInstance.getId(), entry); > } > } > {code} > You can view commit in my fork of framework, if that's ok - I can make a pull > request. > https://github.com/Me1kaa/curator/commit/6e6a34db7d71a84e3a53d284fc94e1fe2e3c10d8 -- This message was sent by Atlassian JIRA (v7.6.3#76005)