You are viewing a plain text version of this content. The canonical link for it is here.
Posted to scm@geronimo.apache.org by jb...@apache.org on 2005/02/23 04:23:56 UTC
svn commit: r154941 - in geronimo/trunk:
modules/deployment/src/java/org/apache/geronimo/deployment/
modules/kernel/src/java/org/apache/geronimo/gbean/
modules/kernel/src/java/org/apache/geronimo/gbean/runtime/
modules/kernel/src/java/org/apache/geronimo/kernel/
modules/kernel/src/java/org/apache/geronimo/kernel/jmx/
modules/kernel/src/java/org/apache/geronimo/kernel/registry/
modules/kernel/src/test/org/apache/geronimo/gbean/
modules/system/src/java/org/apache/geronimo/system/main/
plugins/maven-geronimo-plugin/src/java/org/apache/geronimo/deployment/mavenplugin/
Author: jboynes
Date: Tue Feb 22 19:23:52 2005
New Revision: 154941
URL: http://svn.apache.org/viewcvs?view=rev&rev=154941
Log:
Change GBean registries to key off GBeanName rather than ObjectName.
To reduce impact, the kernel API is currently unchanged and the kernel
maps names accordingly; this will be removed when the API is updated.
The JMXGBeanRegistry now uses an injected MBeanServer rather than
creating one itself; client code that creates this type of registry
has been updated to create the MBeanServer as well.
The GBeanDataRegistry (which holds GBeanData not GBeanInstance instances)
is no longer a GBeanRegistry but its own class. This is only used by
DeploymentContext.
Modified:
geronimo/trunk/modules/deployment/src/java/org/apache/geronimo/deployment/DeploymentContext.java
geronimo/trunk/modules/deployment/src/java/org/apache/geronimo/deployment/GBeanDataRegistry.java
geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/gbean/GBeanName.java
geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/gbean/runtime/GBeanInstance.java
geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/Kernel.java
geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/jmx/JMXGBeanRegistry.java
geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/AbstractGBeanRegistry.java
geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/BasicGBeanRegistry.java
geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/GBeanRegistry.java
geronimo/trunk/modules/kernel/src/test/org/apache/geronimo/gbean/GBeanNameTest.java
geronimo/trunk/modules/system/src/java/org/apache/geronimo/system/main/Daemon.java
geronimo/trunk/plugins/maven-geronimo-plugin/src/java/org/apache/geronimo/deployment/mavenplugin/StartServer.java
Modified: geronimo/trunk/modules/deployment/src/java/org/apache/geronimo/deployment/DeploymentContext.java
URL: http://svn.apache.org/viewcvs/geronimo/trunk/modules/deployment/src/java/org/apache/geronimo/deployment/DeploymentContext.java?view=diff&r1=154940&r2=154941
==============================================================================
--- geronimo/trunk/modules/deployment/src/java/org/apache/geronimo/deployment/DeploymentContext.java (original)
+++ geronimo/trunk/modules/deployment/src/java/org/apache/geronimo/deployment/DeploymentContext.java Tue Feb 22 19:23:52 2005
@@ -113,7 +113,7 @@
throw new AssertionError();
}
- gbeans.setDefaultDomain(domain);
+// gbeans.setDefaultDomain(domain);
if (kernel != null && parentID != null) {
ConfigurationManager configurationManager = kernel.getConfigurationManager();
Modified: geronimo/trunk/modules/deployment/src/java/org/apache/geronimo/deployment/GBeanDataRegistry.java
URL: http://svn.apache.org/viewcvs/geronimo/trunk/modules/deployment/src/java/org/apache/geronimo/deployment/GBeanDataRegistry.java?view=diff&r1=154940&r2=154941
==============================================================================
--- geronimo/trunk/modules/deployment/src/java/org/apache/geronimo/deployment/GBeanDataRegistry.java (original)
+++ geronimo/trunk/modules/deployment/src/java/org/apache/geronimo/deployment/GBeanDataRegistry.java Tue Feb 22 19:23:52 2005
@@ -16,36 +16,34 @@
*/
package org.apache.geronimo.deployment;
-import java.util.Set;
import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.HashSet;
+import java.util.Iterator;
import javax.management.ObjectName;
import org.apache.geronimo.gbean.GBeanData;
+import org.apache.geronimo.gbean.GBeanName;
import org.apache.geronimo.kernel.GBeanNotFoundException;
-import org.apache.geronimo.kernel.registry.AbstractGBeanRegistry;
/**
* @version $Rev: $ $Date: $
*/
-public class GBeanDataRegistry extends AbstractGBeanRegistry {
-
- public void setDefaultDomain(String defaultDomain) {
- this.defaultDomainName = defaultDomain;
- }
+public class GBeanDataRegistry {
+ private final Map registry = new HashMap();
public void preregister(ObjectName name) {
- register(name, null);
+ registry.put(name, null);
}
public void register(GBeanData gbean) {
- register(gbean.getName(), gbean);
+ registry.put(gbean.getName(), gbean);
}
- public GBeanData getGBeanInstance(ObjectName name) throws GBeanNotFoundException {
- GBeanData gbeanData;
- synchronized (this) {
- gbeanData = (GBeanData) registry.get(name);
- }
+ public synchronized GBeanData getGBeanInstance(ObjectName name) throws GBeanNotFoundException {
+ GBeanData gbeanData = (GBeanData) registry.get(name);
if (gbeanData == null) {
throw new GBeanNotFoundException(name.getCanonicalName());
}
@@ -61,4 +59,15 @@
}
+ public Set listGBeans(ObjectName pattern) {
+ Set result = new HashSet();
+ for (Iterator i = registry.entrySet().iterator(); i.hasNext();) {
+ Map.Entry entry = (Map.Entry) i.next();
+ ObjectName name = (ObjectName) entry.getKey();
+ if (pattern.apply(name)) {
+ result.add(name);
+ }
+ }
+ return result;
+ }
}
Modified: geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/gbean/GBeanName.java
URL: http://svn.apache.org/viewcvs/geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/gbean/GBeanName.java?view=diff&r1=154940&r2=154941
==============================================================================
--- geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/gbean/GBeanName.java (original)
+++ geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/gbean/GBeanName.java Tue Feb 22 19:23:52 2005
@@ -19,8 +19,11 @@
import java.io.Serializable;
import java.util.Arrays;
import java.util.HashMap;
+import java.util.Hashtable;
import java.util.Iterator;
import java.util.Map;
+import javax.management.MalformedObjectNameException;
+import javax.management.ObjectName;
/**
@@ -180,5 +183,24 @@
private Object readResolve() {
return new GBeanName(name);
+ }
+
+ // utility methods to support conversion from ObjectName to GBeanName
+
+ /**
+ * @deprecated
+ */
+ public ObjectName getObjectName() throws MalformedObjectNameException {
+ return new ObjectName(domain, new Hashtable(props));
+ }
+
+ /**
+ * @deprecated
+ */
+ public GBeanName(ObjectName name) {
+ this.name = name.toString();
+ this.domain = name.getDomain();
+ this.props = new HashMap(name.getKeyPropertyList());
+ this.hashCode = domain.hashCode() + 37 * props.hashCode();
}
}
Modified: geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/gbean/runtime/GBeanInstance.java
URL: http://svn.apache.org/viewcvs/geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/gbean/runtime/GBeanInstance.java?view=diff&r1=154940&r2=154941
==============================================================================
--- geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/gbean/runtime/GBeanInstance.java (original)
+++ geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/gbean/runtime/GBeanInstance.java Tue Feb 22 19:23:52 2005
@@ -1237,10 +1237,17 @@
}
+ public boolean equals(Object obj) {
+ if (obj == this) return true;
+ if (obj instanceof GBeanInstance == false) return false;
+ return objectName.equals(((GBeanInstance)obj).objectName);
+ }
+
+ public int hashCode() {
+ return objectName.hashCode();
+ }
+
public String toString() {
- if (objectName == null) {
- return super.toString();
- }
return objectName.toString();
}
}
Modified: geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/Kernel.java
URL: http://svn.apache.org/viewcvs/geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/Kernel.java?view=diff&r1=154940&r2=154941
==============================================================================
--- geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/Kernel.java (original)
+++ geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/Kernel.java Tue Feb 22 19:23:52 2005
@@ -37,6 +37,7 @@
import org.apache.commons.logging.LogFactory;
import org.apache.geronimo.gbean.GBeanData;
import org.apache.geronimo.gbean.GBeanInfo;
+import org.apache.geronimo.gbean.GBeanName;
import org.apache.geronimo.gbean.runtime.GBeanInstance;
import org.apache.geronimo.kernel.config.Configuration;
import org.apache.geronimo.kernel.config.ConfigurationManager;
@@ -290,12 +291,12 @@
}
public Object getAttribute(ObjectName objectName, String attributeName) throws GBeanNotFoundException, NoSuchAttributeException, Exception {
- GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(objectName);
+ GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(new GBeanName(objectName));
return gbeanInstance.getAttribute(attributeName);
}
public void setAttribute(ObjectName objectName, String attributeName, Object attributeValue) throws GBeanNotFoundException, NoSuchAttributeException, Exception {
- GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(objectName);
+ GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(new GBeanName(objectName));
gbeanInstance.setAttribute(attributeName, attributeValue);
}
@@ -304,21 +305,21 @@
}
public Object invoke(ObjectName objectName, String methodName, Object[] args, String[] types) throws GBeanNotFoundException, NoSuchOperationException, InternalKernelException, Exception {
- GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(objectName);
+ GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(new GBeanName(objectName));
return gbeanInstance.invoke(methodName, args, types);
}
public boolean isLoaded(ObjectName name) {
- return gbeanRegistry.isRegistered(name);
+ return gbeanRegistry.isRegistered(new GBeanName(name));
}
public GBeanInfo getGBeanInfo(ObjectName name) throws GBeanNotFoundException {
- GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(name);
+ GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(new GBeanName(name));
return gbeanInstance.getGBeanInfo();
}
public GBeanData getGBeanData(ObjectName name) throws GBeanNotFoundException, InternalKernelException {
- GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(name);
+ GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(new GBeanName(name));
return gbeanInstance.getGBeanData();
}
@@ -339,28 +340,37 @@
}
public void startGBean(ObjectName name) throws GBeanNotFoundException, InternalKernelException, IllegalStateException {
- GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(name);
+ GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(new GBeanName(name));
gbeanInstance.start();
}
public void startRecursiveGBean(ObjectName name) throws GBeanNotFoundException, InternalKernelException, IllegalStateException {
- GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(name);
+ GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(new GBeanName(name));
gbeanInstance.startRecursive();
}
public void stopGBean(ObjectName name) throws GBeanNotFoundException, InternalKernelException, IllegalStateException {
- GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(name);
+ GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(new GBeanName(name));
gbeanInstance.stop();
}
public void unloadGBean(ObjectName name) throws GBeanNotFoundException, InternalKernelException, IllegalStateException {
- GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(name);
+ GBeanName gbeanName = new GBeanName(name);
+ GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(gbeanName);
gbeanInstance.die();
- gbeanRegistry.unregister(name);
+ gbeanRegistry.unregister(gbeanName);
}
public Set listGBeans(ObjectName pattern) {
- return gbeanRegistry.listGBeans(pattern);
+ String domain = (pattern == null || pattern.isDomainPattern()) ? null : pattern.getDomain();
+ Map props = pattern == null ? null : pattern.getKeyPropertyList();
+ Set gbeans = gbeanRegistry.listGBeans(domain, props);
+ Set result = new HashSet(gbeans.size());
+ for (Iterator i = gbeans.iterator(); i.hasNext();) {
+ GBeanInstance instance = (GBeanInstance) i.next();
+ result.add(instance.getObjectNameObject());
+ }
+ return result;
}
public Set listGBeans(Set patterns) {
@@ -408,7 +418,7 @@
GBeanInstance gbeanInstance = null;
try {
ObjectName configName = Configuration.getConfigurationObjectName(configID);
- gbeanInstance = gbeanRegistry.getGBeanInstance(configName);
+ gbeanInstance = gbeanRegistry.getGBeanInstance(new GBeanName(configName));
} catch (MalformedObjectNameException e) {
throw new NoSuchConfigException(e);
} catch (GBeanNotFoundException e) {
@@ -537,7 +547,7 @@
// ignore
}
try {
- gbeanRegistry.unregister(CONFIGURATION_MANAGER_NAME);
+ gbeanRegistry.unregister(new GBeanName(CONFIGURATION_MANAGER_NAME));
} catch (Exception e) {
// ignore
}
@@ -550,7 +560,7 @@
}
public ClassLoader getClassLoaderFor(ObjectName name) throws GBeanNotFoundException {
- GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(name);
+ GBeanInstance gbeanInstance = gbeanRegistry.getGBeanInstance(new GBeanName(name));
return gbeanInstance.getClassLoader();
}
Modified: geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/jmx/JMXGBeanRegistry.java
URL: http://svn.apache.org/viewcvs/geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/jmx/JMXGBeanRegistry.java?view=diff&r1=154940&r2=154941
==============================================================================
--- geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/jmx/JMXGBeanRegistry.java (original)
+++ geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/jmx/JMXGBeanRegistry.java Tue Feb 22 19:23:52 2005
@@ -17,106 +17,94 @@
package org.apache.geronimo.kernel.jmx;
import java.util.HashMap;
-import java.util.Set;
+import java.util.Iterator;
import javax.management.InstanceAlreadyExistsException;
import javax.management.InstanceNotFoundException;
import javax.management.JMException;
import javax.management.JMRuntimeException;
import javax.management.MBeanInfo;
import javax.management.MBeanServer;
-import javax.management.MBeanServerFactory;
import javax.management.ObjectName;
+import org.apache.geronimo.gbean.GBeanName;
import org.apache.geronimo.gbean.runtime.GBeanInstance;
import org.apache.geronimo.gbean.runtime.LifecycleBroadcaster;
import org.apache.geronimo.kernel.GBeanAlreadyExistsException;
import org.apache.geronimo.kernel.GBeanNotFoundException;
-import org.apache.geronimo.kernel.registry.GBeanRegistry;
import org.apache.geronimo.kernel.InternalKernelException;
import org.apache.geronimo.kernel.Kernel;
import org.apache.geronimo.kernel.lifecycle.LifecycleListener;
+import org.apache.geronimo.kernel.registry.AbstractGBeanRegistry;
/**
+ * An implementation of GBeanRegistry that also registers the GBeans with a JMX MBeanServer.
+ *
* @version $Rev$ $Date$
*/
-public class JMXGBeanRegistry implements GBeanRegistry {
+public class JMXGBeanRegistry extends AbstractGBeanRegistry {
private final HashMap registry = new HashMap();
private Kernel kernel;
- private MBeanServer mbServer;
+ private final MBeanServer mbServer;
+
+ public JMXGBeanRegistry(MBeanServer mbServer) {
+ this.mbServer = mbServer;
+ }
public void start(Kernel kernel) {
+ super.start(kernel);
this.kernel = kernel;
- mbServer = MBeanServerFactory.createMBeanServer(kernel.getKernelName());
}
- public void stop() {
- MBeanServerFactory.releaseMBeanServer(mbServer);
+ public synchronized void stop() {
+ this.kernel = null;
- // todo destroy instances
- synchronized(this) {
- registry.clear();
+ // unregister all our GBean from the MBeanServer
+ for (Iterator i = registry.keySet().iterator(); i.hasNext();) {
+ GBeanName name = (GBeanName) i.next();
+ try {
+ mbServer.unregisterMBean(name.getObjectName());
+ } catch (Exception e) {
+ // ignore
+ }
}
+ super.stop();
}
- public MBeanServer getMBeanServer() {
- return mbServer;
- }
-
- public synchronized boolean isRegistered(ObjectName name) {
- return registry.containsKey(name);
- }
-
- public void register(GBeanInstance gbeanInstance) throws GBeanAlreadyExistsException, InternalKernelException {
+ public void register(GBeanInstance gbeanInstance) throws GBeanAlreadyExistsException {
+ // create an MBean to wrap the plain GBean
ObjectName name = gbeanInstance.getObjectNameObject();
MBeanInfo mbeanInfo = JMXUtil.toMBeanInfo(gbeanInstance.getGBeanInfo());
GBeanMBean gbeanMBean = new GBeanMBean(kernel, name, mbeanInfo);
+
+ // register the MBean with the JMX MBeanServer
try {
mbServer.registerMBean(gbeanMBean, name);
} catch (InstanceAlreadyExistsException e) {
- throw new GBeanAlreadyExistsException("A GBean is alreayd registered witht then name " + name);
+ throw new GBeanAlreadyExistsException("An MBean is already registered under the name " + name);
} catch (Exception e) {
throw new InternalKernelException("Error loading GBean " + name.getCanonicalName(), unwrapJMException(e));
}
- synchronized (this) {
- registry.put(name, gbeanInstance);
- }
+ super.register(gbeanInstance);
+ // todo when can we get rid if this?
+ // fire the loaded event from the gbeanMBean.. it was already fired from the GBeanInstance when it was created
kernel.getLifecycleMonitor().addLifecycleListener(new LifecycleBridge(gbeanMBean), name);
-
- // fire the loaded event from the gbeanMBean.. it was already fired from
- // the GBeanInstance when it was created
- gbeanMBean.fireLoadedEvent();
+ gbeanMBean.fireLoadedEvent();
}
- public void unregister(ObjectName name) throws GBeanNotFoundException, InternalKernelException {
+ public void unregister(GBeanName name) throws GBeanNotFoundException, InternalKernelException {
try {
- mbServer.unregisterMBean(name);
+ ObjectName objectName = name.getObjectName();
+ mbServer.unregisterMBean(objectName);
} catch (InstanceNotFoundException e) {
- throw new GBeanNotFoundException(name.getCanonicalName());
+ // ignore - something else may have unregistered us
+ // if there truely is no GBean then we will catch it below whwn we call the superclass
} catch (Exception e) {
throw new InternalKernelException("Error unloading GBean " + name, unwrapJMException(e));
}
- synchronized (this) {
- registry.remove(name);
- }
- }
-
- public synchronized GBeanInstance getGBeanInstance(ObjectName name) throws GBeanNotFoundException {
- GBeanInstance gbeanInstance = (GBeanInstance) registry.get(name);
- if (gbeanInstance == null) {
- throw new GBeanNotFoundException(name.getCanonicalName());
- }
- return gbeanInstance;
- }
-
- public Set listGBeans(ObjectName pattern) throws InternalKernelException {
- try {
- return mbServer.queryNames(pattern, null);
- } catch (RuntimeException e) {
- throw new InternalKernelException("Error while applying pattern " + pattern, e);
- }
+ super.unregister(name);
}
private Throwable unwrapJMException(Throwable cause) {
Modified: geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/AbstractGBeanRegistry.java
URL: http://svn.apache.org/viewcvs/geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/AbstractGBeanRegistry.java?view=diff&r1=154940&r2=154941
==============================================================================
--- geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/AbstractGBeanRegistry.java (original)
+++ geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/AbstractGBeanRegistry.java Tue Feb 22 19:23:52 2005
@@ -16,196 +16,74 @@
*/
package org.apache.geronimo.kernel.registry;
-import java.util.ArrayList;
-import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
-import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.regex.Pattern;
import javax.management.ObjectName;
+import org.apache.geronimo.gbean.GBeanName;
+import org.apache.geronimo.gbean.runtime.GBeanInstance;
+import org.apache.geronimo.kernel.GBeanAlreadyExistsException;
import org.apache.geronimo.kernel.GBeanNotFoundException;
import org.apache.geronimo.kernel.InternalKernelException;
+import org.apache.geronimo.kernel.Kernel;
/**
- * @version $Rev: $ $Date: $
+ * @version $Rev$ $Date$
*/
-public class AbstractGBeanRegistry {
+public abstract class AbstractGBeanRegistry implements GBeanRegistry {
protected final Map registry = new HashMap();
- protected final Map domainIndex = new HashMap();
- protected String defaultDomainName;
- public void stop() {
- // todo destroy instances
- synchronized (this) {
- registry.clear();
- domainIndex.clear();
- }
+ public void start(Kernel kernel) {
}
- public boolean isRegistered(ObjectName name) {
- synchronized (this) {
- return registry.containsKey(name);
- }
+ public synchronized void stop() {
+ registry.clear();
}
- protected void register(ObjectName name, Object gbean) {
- // do as much work as possible outside of the synchronized block
- String domainName = name.getDomain();
- // convert properties list to a HashMap as it is more efficient then the synchronized Hashtable
- Map properties = new HashMap(name.getKeyPropertyList());
-
- synchronized (this) {
- registry.put(name, gbean);
-
- Map nameToProperties = (Map) domainIndex.get(domainName);
- if (nameToProperties == null) {
- nameToProperties = new HashMap();
- domainIndex.put(domainName, nameToProperties);
- }
- nameToProperties.put(name, properties);
- }
+ public synchronized boolean isRegistered(GBeanName name) {
+ return registry.containsKey(name);
}
- public void unregister(ObjectName name) throws GBeanNotFoundException, InternalKernelException {
- String domainName = name.getDomain();
- synchronized (this) {
- registry.remove(name);
-
- // just leave the an empty nameToProperty map
- Map nameToProperties = (Map) domainIndex.get(domainName);
- if (nameToProperties != null) {
- nameToProperties.remove(name);
- }
+ public synchronized void register(GBeanInstance gbeanInstance) throws GBeanAlreadyExistsException {
+ ObjectName objectName = gbeanInstance.getObjectNameObject();
+ GBeanName name = new GBeanName(objectName);
+ if (registry.containsKey(name)) {
+ throw new GBeanAlreadyExistsException("GBean already registered: " + name);
}
+ registry.put(name, gbeanInstance);
}
- public Set listGBeans(ObjectName pattern) throws InternalKernelException {
- if (pattern == null) {
- synchronized (this) {
- return new HashSet(registry.keySet());
- }
- }
-
- String patternDomain = pattern.getDomain();
- if (patternDomain.length() == 0) {
- patternDomain = defaultDomainName;
- }
-
- // work with a copy of the registry key set
- List nameToProperties;
- if (!pattern.isDomainPattern()) {
- synchronized (this) {
- // create an array list big enough to match all names... extra space is better than resizing
- nameToProperties = new ArrayList(registry.size());
-
- // find we are only matching one specific domain, so
- // just grab it directly from the index
- Map map = (Map) domainIndex.get(patternDomain);
- if (map != null) {
- nameToProperties.addAll(map.entrySet());
- }
- }
- } else if (patternDomain.equals("*")) {
- // this is very commmon, so support it directly
- synchronized (this) {
- // create an array list big enough to match all names... extra space is better than resizing
- nameToProperties = new ArrayList(registry.size());
-
- // find we are matching all domain, so just grab all of them directly
- for (Iterator iterator = domainIndex.values().iterator(); iterator.hasNext();) {
- Map map = (Map) iterator.next();
-
- // we can just copy the entry set directly into the list we don't
- // have to worry about duplicates as the maps are mutually exclusive
- nameToProperties.addAll(map.entrySet());
- }
- }
- } else {
- String perl5Pattern = domainPatternToPerl5(patternDomain);
- Pattern domainPattern = Pattern.compile(perl5Pattern);
-
- synchronized (this) {
- // create an array list big enough to match all names... extra space is better than resizing
- nameToProperties = new ArrayList(registry.size());
-
- // find all of the matching domains
- for (Iterator iterator = domainIndex.entrySet().iterator(); iterator.hasNext();) {
- Map.Entry entry = (Map.Entry) iterator.next();
- String domain = (String) entry.getKey();
- if (domainPattern.matcher(domain).matches()) {
- // we can just copy the entry set directly into the list we don't
- // have to worry about duplicates as the maps are mutually exclusive
- Map map = (Map) entry.getValue();
- nameToProperties.addAll(map.entrySet());
- }
- }
- }
- }
-
- if (nameToProperties.isEmpty()) {
- return Collections.EMPTY_SET;
+ public synchronized void unregister(GBeanName name) throws GBeanNotFoundException, InternalKernelException {
+ if (registry.remove(name) == null) {
+ throw new GBeanNotFoundException("No GBean registered: " + name);
}
+ }
- // convert the pattern property list to a HashMap as it is not synchronized
- Map patternProperties = new HashMap(pattern.getKeyPropertyList());
- patternProperties.remove("*");
- boolean isMatchAll = patternProperties.isEmpty();
- boolean isPropertyPattern = pattern.isPropertyPattern();
-
- Set matchingNames = new HashSet();
- for (Iterator iterator = nameToProperties.iterator(); iterator.hasNext();) {
- Map.Entry entry = (Map.Entry) iterator.next();
- Map properties = (Map) entry.getValue();
-
- if (isMatchAll) {
- matchingNames.add(entry.getKey());
- } else if (isPropertyPattern) {
- if (properties.entrySet().containsAll(patternProperties.entrySet())) {
- matchingNames.add(entry.getKey());
- }
-
- } else {
- if (properties.entrySet().equals(patternProperties.entrySet())) {
- matchingNames.add(entry.getKey());
- }
- }
+ public synchronized GBeanInstance getGBeanInstance(GBeanName name) throws GBeanNotFoundException {
+ GBeanInstance instance = (GBeanInstance) registry.get(name);
+ if (instance == null) {
+ throw new GBeanNotFoundException("No GBean registered: " + name);
}
- return matchingNames;
+ return instance;
}
- private static String domainPatternToPerl5(String pattern) {
- char[] patternCharacters = pattern.toCharArray();
- StringBuffer buffer = new StringBuffer(2 * patternCharacters.length);
- for (int position = 0; position < patternCharacters.length; position++) {
- char character = patternCharacters[position];
- switch (character) {
- case '*':
- // replace '*' with '.*'
- buffer.append(".*");
- break;
- case '?':
- // replace '?' with '.'
- buffer.append('.');
- break;
- default:
- // escape any perl5 characters with '\'
- if (isPerl5MetaCharacter(character)) {
- buffer.append('\\');
- }
- buffer.append(character);
- break;
+ public Set listGBeans(String domain, Map properties) {
+ // fairly dumb implementation that iterates the list of all registered GBeans
+ Map clone;
+ synchronized(this) {
+ clone = new HashMap(registry);
+ }
+ Set result = new HashSet(clone.size());
+ for (Iterator i = clone.entrySet().iterator(); i.hasNext();) {
+ Map.Entry entry = (Map.Entry) i.next();
+ GBeanName name = (GBeanName) entry.getKey();
+ if (name.matches(domain, properties)) {
+ result.add(entry.getValue());
}
}
-
- return buffer.toString();
+ return result;
}
-
- private static boolean isPerl5MetaCharacter(char character) {
- return ("'*?+[]()|^$.{}\\".indexOf(character) >= 0);
- }
-
}
Modified: geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/BasicGBeanRegistry.java
URL: http://svn.apache.org/viewcvs/geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/BasicGBeanRegistry.java?view=diff&r1=154940&r2=154941
==============================================================================
--- geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/BasicGBeanRegistry.java (original)
+++ geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/BasicGBeanRegistry.java Tue Feb 22 19:23:52 2005
@@ -19,6 +19,7 @@
import javax.management.ObjectName;
import org.apache.geronimo.gbean.runtime.GBeanInstance;
+import org.apache.geronimo.gbean.GBeanName;
import org.apache.geronimo.kernel.GBeanAlreadyExistsException;
import org.apache.geronimo.kernel.GBeanNotFoundException;
import org.apache.geronimo.kernel.InternalKernelException;
@@ -27,27 +28,5 @@
/**
* @version $Rev$ $Date$
*/
-public class BasicGBeanRegistry extends AbstractGBeanRegistry implements GBeanRegistry {
-
- public void start(Kernel kernel) {
- this.defaultDomainName = kernel.getKernelName();
- }
-
- public void register(GBeanInstance gbeanInstance) throws GBeanAlreadyExistsException, InternalKernelException {
- ObjectName name = gbeanInstance.getObjectNameObject();
- register(name, gbeanInstance);
-
- }
-
- public GBeanInstance getGBeanInstance(ObjectName name) throws GBeanNotFoundException {
- GBeanInstance gbeanInstance;
- synchronized (this) {
- gbeanInstance = (GBeanInstance) registry.get(name);
- }
- if (gbeanInstance == null) {
- throw new GBeanNotFoundException(name.getCanonicalName());
- }
- return gbeanInstance;
- }
-
+public class BasicGBeanRegistry extends AbstractGBeanRegistry {
}
Modified: geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/GBeanRegistry.java
URL: http://svn.apache.org/viewcvs/geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/GBeanRegistry.java?view=diff&r1=154940&r2=154941
==============================================================================
--- geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/GBeanRegistry.java (original)
+++ geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/GBeanRegistry.java Tue Feb 22 19:23:52 2005
@@ -17,29 +17,71 @@
package org.apache.geronimo.kernel.registry;
import java.util.Set;
+import java.util.Map;
import javax.management.ObjectName;
+import org.apache.geronimo.gbean.GBeanName;
import org.apache.geronimo.gbean.runtime.GBeanInstance;
-import org.apache.geronimo.kernel.Kernel;
import org.apache.geronimo.kernel.GBeanAlreadyExistsException;
-import org.apache.geronimo.kernel.InternalKernelException;
import org.apache.geronimo.kernel.GBeanNotFoundException;
+import org.apache.geronimo.kernel.Kernel;
/**
+ * Interface implemented by Registries that a Kernel can use to store and retrieve GBeanInstances.
* @version $Rev$ $Date$
*/
public interface GBeanRegistry {
+ /**
+ * Start the registry and associate it with a kernel.
+ *
+ * @param kernel the kernel to associate with
+ */
void start(Kernel kernel);
+ /**
+ * Shut down the registry and unregister any GBeans
+ */
void stop();
- boolean isRegistered(ObjectName name);
-
- void register(GBeanInstance gbeanInstance) throws GBeanAlreadyExistsException, InternalKernelException;
-
- void unregister(ObjectName name) throws GBeanNotFoundException, InternalKernelException;
-
- GBeanInstance getGBeanInstance(ObjectName name) throws GBeanNotFoundException;
-
- Set listGBeans(ObjectName pattern) throws InternalKernelException;
+ /**
+ * See if there is a GBean registered with a specific name.
+ *
+ * @param name the name of the GBean to check for
+ * @return true if there is a GBean registered with that name
+ */
+ boolean isRegistered(GBeanName name);
+
+ /**
+ * Register a GBean instance.
+ *
+ * @param gbeanInstance the GBean to register
+ * @throws GBeanAlreadyExistsException if there is already a GBean registered with the instance's name
+ */
+ void register(GBeanInstance gbeanInstance) throws GBeanAlreadyExistsException;
+
+ /**
+ * Unregister a GBean instance.
+ *
+ * @param name the name of the GBean to unregister
+ * @throws GBeanNotFoundException if there is no GBean registered with the supplied name
+ */
+ void unregister(GBeanName name) throws GBeanNotFoundException;
+
+ /**
+ * Return the GBeanInstance registered with the supplied name.
+ *
+ * @param name the name of the instance to return
+ * @return the GBeanInstance
+ * @throws GBeanNotFoundException if there is no GBean registered with the supplied name
+ */
+ GBeanInstance getGBeanInstance(GBeanName name) throws GBeanNotFoundException;
+
+ /**
+ * Search the registry for GBeans matching a name pattern.
+ *
+ * @param domain the domain to query in; null indicates all
+ * @param properties the properties the GBeans must have
+ * @return an unordered Set<GBeanInstance> of GBeans that matched the pattern
+ */
+ Set listGBeans(String domain, Map properties);
}
Modified: geronimo/trunk/modules/kernel/src/test/org/apache/geronimo/gbean/GBeanNameTest.java
URL: http://svn.apache.org/viewcvs/geronimo/trunk/modules/kernel/src/test/org/apache/geronimo/gbean/GBeanNameTest.java?view=diff&r1=154940&r2=154941
==============================================================================
--- geronimo/trunk/modules/kernel/src/test/org/apache/geronimo/gbean/GBeanNameTest.java (original)
+++ geronimo/trunk/modules/kernel/src/test/org/apache/geronimo/gbean/GBeanNameTest.java Tue Feb 22 19:23:52 2005
@@ -67,7 +67,7 @@
public void testInvalidNames() {
try {
- new GBeanName(null);
+ new GBeanName((String) null);
fail();
} catch (NullPointerException e) {
}
Modified: geronimo/trunk/modules/system/src/java/org/apache/geronimo/system/main/Daemon.java
URL: http://svn.apache.org/viewcvs/geronimo/trunk/modules/system/src/java/org/apache/geronimo/system/main/Daemon.java?view=diff&r1=154940&r2=154941
==============================================================================
--- geronimo/trunk/modules/system/src/java/org/apache/geronimo/system/main/Daemon.java (original)
+++ geronimo/trunk/modules/system/src/java/org/apache/geronimo/system/main/Daemon.java Tue Feb 22 19:23:52 2005
@@ -27,6 +27,7 @@
import java.util.List;
import java.util.Set;
import javax.management.ObjectName;
+import javax.management.MBeanServerFactory;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -132,7 +133,7 @@
}
// build a jms kernel
- final Kernel kernel = new Kernel("geronimo", new JMXGBeanRegistry());
+ final Kernel kernel = new Kernel("geronimo", new JMXGBeanRegistry(MBeanServerFactory.createMBeanServer("geronimo")));
// boot the kernel
try {
Modified: geronimo/trunk/plugins/maven-geronimo-plugin/src/java/org/apache/geronimo/deployment/mavenplugin/StartServer.java
URL: http://svn.apache.org/viewcvs/geronimo/trunk/plugins/maven-geronimo-plugin/src/java/org/apache/geronimo/deployment/mavenplugin/StartServer.java?view=diff&r1=154940&r2=154941
==============================================================================
--- geronimo/trunk/plugins/maven-geronimo-plugin/src/java/org/apache/geronimo/deployment/mavenplugin/StartServer.java (original)
+++ geronimo/trunk/plugins/maven-geronimo-plugin/src/java/org/apache/geronimo/deployment/mavenplugin/StartServer.java Tue Feb 22 19:23:52 2005
@@ -26,9 +26,11 @@
import java.util.List;
import java.util.StringTokenizer;
import javax.management.ObjectName;
+import javax.management.MBeanServerFactory;
import org.apache.geronimo.gbean.GBeanData;
import org.apache.geronimo.kernel.Kernel;
+import org.apache.geronimo.kernel.registry.BasicGBeanRegistry;
import org.apache.geronimo.kernel.jmx.JMXGBeanRegistry;
import org.apache.geronimo.kernel.config.ConfigurationManager;
import org.apache.geronimo.kernel.log.GeronimoLogging;
@@ -106,7 +108,7 @@
}
// build a basic kernel without a configuration-store, our configuration store is
- Kernel kernel = new Kernel(getKernelName(), new JMXGBeanRegistry());
+ Kernel kernel = new Kernel(getKernelName(), new BasicGBeanRegistry());
kernel.boot();
ConfigurationManager configurationManager = kernel.getConfigurationManager();
Re: Signatrue change of GBeanRegistry.listGBeans [Re: svn commit: r154941]
Posted by Dain Sundstrom <ds...@gluecode.com>.
On Feb 23, 2005, at 6:35 PM, Jeremy Boynes wrote:
> Well you cut everything else from the email ...
Sorry missed that:
> Same with splitting the name - that's not removing functionality, its
> refactoring the API to make it easier to use by eliminating the
> overlap between values and patterns (given, even in JMX, patterns
> proved inadequate and lead to the introduction of QueryExp).
Well I guess it depends on how you are proposing to implement it. If
it involves a sub class, then I guess it doesn't matter. If it does
not involve a subclass, then it means I can not handle an object name
and a pattern using the same code. Anyway, I still believe the burden
is on you to show that this is simpler then having a GBeanName
supporting patterns out of the box (just like object name).
>> Saying that "removing complexity is good" is like saying "puppies are
>> cute". Yes we all agree, but that does not prove that this feature
>> results in complexity, and that removing it will result in less
>> complexity. Complexity is a hydra, you chop off one head and it
>> reappears somewhere else.
>> I personally do not find the domain query complex and we already have
>> a fully working implementation in the non-jmx registry. I do not
>> think that removing it will make this class noticeably simpler, but
>> maybe you are thinking of some other code.
>
> I'm thinking of the code that checked if the domain was pattern
> (involving a String scan), did a character substitution to convert it
> to a Perl regex (multiple String scans), and then applied that Pattern
> to all values (some state machine in the VM).
>
> See the code at the end of this mail or at:
> http://svn.apache.org/viewcvs.cgi/geronimo/trunk/modules/kernel/src/
> java/org/apache/geronimo/kernel/registry/AbstractGBeanRegistry.java?
> rev=151106&view=markup
>
> Or, which delegated to MBeanServer.queryMBeans() and believe me I know
> the gyrations that goes through (never mind you're querying all
> registered MBeans not just GBeans) including a variety of security
> checks.
>
> Compared to the revised implementation:
> if (domain != null) {
> if (!this.domain.equals(domain)) {
> return false;
> }
> }
I just looked at the code again, and it is simple to me and well
documented. The code is optimized for the case where we are not using
a pattern, and for the case where we are using a pattern, I believe the
implementation is reasonably fast. If not, that can be corrected as an
internal detail of the code. Also, this feature is isolated to *one*
class in Geronimo, so I really don't see this nasty complexity you want
to cut out.
>> Is there some other reason you want to remove these feature?
>
> Nope - the previous implementation was needlessly complex given it was
> not actually used.
In that case, please leave it alone.
-dain
Re: Signatrue change of GBeanRegistry.listGBeans [Re: svn commit:
r154941]
Posted by Jeremy Boynes <jb...@apache.org>.
Dain Sundstrom wrote:
> On Feb 23, 2005, at 6:00 PM, Jeremy Boynes wrote:
>
>> Dain Sundstrom wrote:
>>
>>> No. I'm saying we have a status quo that people understand and code
>>> to. You are arguing to change the this so, I believe the burden is
>>> on you to prove that we are better off removing the features.
>>
>>
>> You yourself said that no-one is using it, so removing the complexity
>> and simplifying the implementation is normally considered a good thing.
>
>
> My comment was not just about domain queries, it also addressed the
> split of names and patterns, which you comment does not address.
>
Well you cut everything else from the email ...
> Saying that "removing complexity is good" is like saying "puppies are
> cute". Yes we all agree, but that does not prove that this feature
> results in complexity, and that removing it will result in less
> complexity. Complexity is a hydra, you chop off one head and it
> reappears somewhere else.
>
> I personally do not find the domain query complex and we already have a
> fully working implementation in the non-jmx registry. I do not think
> that removing it will make this class noticeably simpler, but maybe you
> are thinking of some other code.
I'm thinking of the code that checked if the domain was pattern
(involving a String scan), did a character substitution to convert it to
a Perl regex (multiple String scans), and then applied that Pattern to
all values (some state machine in the VM).
See the code at the end of this mail or at:
http://svn.apache.org/viewcvs.cgi/geronimo/trunk/modules/kernel/src/java/org/apache/geronimo/kernel/registry/AbstractGBeanRegistry.java?rev=151106&view=markup
Or, which delegated to MBeanServer.queryMBeans() and believe me I know
the gyrations that goes through (never mind you're querying all
registered MBeans not just GBeans) including a variety of security checks.
Compared to the revised implementation:
if (domain != null) {
if (!this.domain.equals(domain)) {
return false;
}
}
>
> Is there some other reason you want to remove these feature?
>
Nope - the previous implementation was needlessly complex given it was
not actually used.
KISS
--
Jeremy
Code from previous version:
String patternDomain = pattern.getDomain();
if (patternDomain.length() == 0) {
patternDomain = defaultDomainName;
}
// work with a copy of the registry key set
List nameToProperties;
if (!pattern.isDomainPattern()) {
synchronized (this) {
// create an array list big enough to match all
names... extra space is better than resizing
nameToProperties = new ArrayList(registry.size());
// find we are only matching one specific domain, so
// just grab it directly from the index
Map map = (Map) domainIndex.get(patternDomain);
if (map != null) {
nameToProperties.addAll(map.entrySet());
}
}
} else if (patternDomain.equals("*")) {
// this is very commmon, so support it directly
synchronized (this) {
// create an array list big enough to match all
names... extra space is better than resizing
nameToProperties = new ArrayList(registry.size());
// find we are matching all domain, so just grab all of
them directly
for (Iterator iterator =
domainIndex.values().iterator(); iterator.hasNext();) {
Map map = (Map) iterator.next();
// we can just copy the entry set directly into the
list we don't
// have to worry about duplicates as the maps are
mutually exclusive
nameToProperties.addAll(map.entrySet());
}
}
} else {
String perl5Pattern = domainPatternToPerl5(patternDomain);
Pattern domainPattern = Pattern.compile(perl5Pattern);
synchronized (this) {
// create an array list big enough to match all
names... extra space is better than resizing
nameToProperties = new ArrayList(registry.size());
// find all of the matching domains
for (Iterator iterator =
domainIndex.entrySet().iterator(); iterator.hasNext();) {
Map.Entry entry = (Map.Entry) iterator.next();
String domain = (String) entry.getKey();
if (domainPattern.matcher(domain).matches()) {
// we can just copy the entry set directly into
the list we don't
// have to worry about duplicates as the maps
are mutually exclusive
Map map = (Map) entry.getValue();
nameToProperties.addAll(map.entrySet());
}
}
}
}
private static String domainPatternToPerl5(String pattern) {
char[] patternCharacters = pattern.toCharArray();
StringBuffer buffer = new StringBuffer(2 *
patternCharacters.length);
for (int position = 0; position < patternCharacters.length;
position++) {
char character = patternCharacters[position];
switch (character) {
case '*':
// replace '*' with '.*'
buffer.append(".*");
break;
case '?':
// replace '?' with '.'
buffer.append('.');
break;
default:
// escape any perl5 characters with '\'
if (isPerl5MetaCharacter(character)) {
buffer.append('\\');
}
buffer.append(character);
break;
}
}
return buffer.toString();
}
private static boolean isPerl5MetaCharacter(char character) {
return ("'*?+[]()|^$.{}\\".indexOf(character) >= 0);
}
Re: Signatrue change of GBeanRegistry.listGBeans [Re: svn commit: r154941]
Posted by Dain Sundstrom <ds...@gluecode.com>.
On Feb 23, 2005, at 6:00 PM, Jeremy Boynes wrote:
> Dain Sundstrom wrote:
>> No. I'm saying we have a status quo that people understand and code
>> to. You are arguing to change the this so, I believe the burden is
>> on you to prove that we are better off removing the features.
>
> You yourself said that no-one is using it, so removing the complexity
> and simplifying the implementation is normally considered a good
> thing.
My comment was not just about domain queries, it also addressed the
split of names and patterns, which you comment does not address.
Saying that "removing complexity is good" is like saying "puppies are
cute". Yes we all agree, but that does not prove that this feature
results in complexity, and that removing it will result in less
complexity. Complexity is a hydra, you chop off one head and it
reappears somewhere else.
I personally do not find the domain query complex and we already have a
fully working implementation in the non-jmx registry. I do not think
that removing it will make this class noticeably simpler, but maybe you
are thinking of some other code.
Is there some other reason you want to remove these feature?
-dain
Re: Signatrue change of GBeanRegistry.listGBeans [Re: svn commit:
r154941]
Posted by Jeremy Boynes <jb...@apache.org>.
Dain Sundstrom wrote:
> On Feb 23, 2005, at 5:42 PM, Jeremy Boynes wrote:
>
>> Dain Sundstrom wrote:
>>
>>>> Nope, as I said, I removed /partial/ wildcards - stuff like
>>>> "test?omain" - full wildcards (i.e. "*" or no value) operate as
>>>> before. Do you really see a need to keep that, and if so wouldn't it
>>>> be better in a more flexible query mechanism similar to JMX's QueryExp?
>>>
>>> I personally dislike the JMX QueryExp objects, and the Kernel has
>>> never supported them. If you want to explore a QueryExp system
>>> because you find it useful, please do. I like the current features
>>> of the ObjectName queries and don't want to have that functionality
>>> removed.
>>
>>
>> So this is based on a personal preference for a rather arcane
>> mechanism that is not actually used by anyone rather than any real
>> technical reason?
>
>
> No. I'm saying we have a status quo that people understand and code to.
> You are arguing to change the this so, I believe the burden is on you
> to prove that we are better off removing the features.
>
You yourself said that no-one is using it, so removing the complexity
and simplifying the implementation is normally considered a good thing.
--
Jeremy
Re: Signatrue change of GBeanRegistry.listGBeans [Re: svn commit: r154941]
Posted by Dain Sundstrom <ds...@gluecode.com>.
On Feb 23, 2005, at 5:42 PM, Jeremy Boynes wrote:
> Dain Sundstrom wrote:
>
>>> Nope, as I said, I removed /partial/ wildcards - stuff like
>>> "test?omain" - full wildcards (i.e. "*" or no value) operate as
>>> before. Do you really see a need to keep that, and if so wouldn't it
>>> be better in a more flexible query mechanism similar to JMX's
>>> QueryExp?
>> I personally dislike the JMX QueryExp objects, and the Kernel has
>> never supported them. If you want to explore a QueryExp system
>> because you find it useful, please do. I like the current features
>> of the ObjectName queries and don't want to have that functionality
>> removed.
>
> So this is based on a personal preference for a rather arcane
> mechanism that is not actually used by anyone rather than any real
> technical reason?
No. I'm saying we have a status quo that people understand and code to.
You are arguing to change the this so, I believe the burden is on you
to prove that we are better off removing the features.
-dain
Re: Signatrue change of GBeanRegistry.listGBeans [Re: svn commit:
r154941]
Posted by Jeremy Boynes <jb...@apache.org>.
Dain Sundstrom wrote:
>> Nope, as I said, I removed /partial/ wildcards - stuff like
>> "test?omain" - full wildcards (i.e. "*" or no value) operate as
>> before. Do you really see a need to keep that, and if so wouldn't it
>> be better in a more flexible query mechanism similar to JMX's QueryExp?
>
>
> I personally dislike the JMX QueryExp objects, and the Kernel has never
> supported them. If you want to explore a QueryExp system because you
> find it useful, please do. I like the current features of the
> ObjectName queries and don't want to have that functionality removed.
>
So this is based on a personal preference for a rather arcane mechanism
that is not actually used by anyone rather than any real technical reason?
>> I treat a /missing/ domain as a wildcard, which is exactly what we did
>> before. An empty domain as in ":x=x" is not a wildcard but an exact
>> match to the domain "". It specifically does not match the default
>> domain unless that just happens to have been set to "" as well (and
>> the default default domain is "DefaultDomain").
>
>
> From, my understanding, and I could be wrong. The name ":x=x" should be
> queried as "<default-domain>:x=x" according to JMX. If I was not doing
> that in the non-jmx based registry it was a bug.
>
You might indeed be wrong. I just got through fixing issues here in MX4J.
>>> Anyway, here is my formal -1 roadblock.
>>
>>
>> To what and why?
>
>
> Removal of domain query functionality and splitting of the name and
> pattern. I don't see any reason to remove this functionality.
>
Removal of a feature that adds complexity for little value that we both
agree no-one is using? How is this different from removing
WaitingException?
Same with splitting the name - that's not removing functionality, its
refactoring the API to make it easier to use by eliminating the overlap
between values and patterns (given, even in JMX, patterns proved
inadequate and lead to the introduction of QueryExp).
KISS, right?
--
Jeremy
Re: Signatrue change of GBeanRegistry.listGBeans [Re: svn commit: r154941]
Posted by Dain Sundstrom <ds...@gluecode.com>.
On Feb 23, 2005, at 11:03 AM, Jeremy Boynes wrote:
> Dain Sundstrom wrote:
>> On Feb 23, 2005, at 8:45 AM, Jeremy Boynes wrote:
>>> Dain Sundstrom wrote:
>>>
>>>> On Feb 22, 2005, at 7:23 PM, jboynes@apache.org wrote:
>>>>
>>>>>
>>>>> public Set listGBeans(ObjectName pattern) {
>>>>> - return gbeanRegistry.listGBeans(pattern);
>>>>> + String domain = (pattern == null ||
>>>>> pattern.isDomainPattern()) ? null : pattern.getDomain();
>>>>> + Map props = pattern == null ? null :
>>>>> pattern.getKeyPropertyList();
>>>>> + Set gbeans = gbeanRegistry.listGBeans(domain, props);
>>>>> + Set result = new HashSet(gbeans.size());
>>>>> + for (Iterator i = gbeans.iterator(); i.hasNext();) {
>>>>> + GBeanInstance instance = (GBeanInstance) i.next();
>>>>> + result.add(instance.getObjectNameObject());
>>>>> + }
>>>>> + return result;
>>>>> }
>>>>
>>>> On just a cursory review of this commit, it appears that you have
>>>> changed the way object name queries work. Before making any other
>>>> changes to this code, can you please explain the changes you want
>>>> to make, so we can discuss the impact? I am very nervous about any
>>>> changes to this code.
>>>
>>>
>>> Perhaps on more detailed examination you'll realize that the only
>>> thing changed is that we no longer support partially wildcarded
>>> domain patterns, something that we never used and was pretty
>>> pointless.
>> So you have removed the feature supporting domain wild cards. I know
>> we currently don't use them, but don't see any reason to remove them
>> either. Also I belive that you have changed the meaning of an empty
>> domain name. In JMX it means match the default domain only, which is
>> something I want to begin using in our plans and matching code.
>
> Nope, as I said, I removed /partial/ wildcards - stuff like
> "test?omain" - full wildcards (i.e. "*" or no value) operate as
> before. Do you really see a need to keep that, and if so wouldn't it
> be better in a more flexible query mechanism similar to JMX's
> QueryExp?
I personally dislike the JMX QueryExp objects, and the Kernel has never
supported them. If you want to explore a QueryExp system because you
find it useful, please do. I like the current features of the
ObjectName queries and don't want to have that functionality removed.
> I treat a /missing/ domain as a wildcard, which is exactly what we did
> before. An empty domain as in ":x=x" is not a wildcard but an exact
> match to the domain "". It specifically does not match the default
> domain unless that just happens to have been set to "" as well (and
> the default default domain is "DefaultDomain").
From, my understanding, and I could be wrong. The name ":x=x" should
be queried as "<default-domain>:x=x" according to JMX. If I was not
doing that in the non-jmx based registry it was a bug.
>> Anyway, here is my formal -1 roadblock.
>
> To what and why?
Removal of domain query functionality and splitting of the name and
pattern. I don't see any reason to remove this functionality.
-dain
Re: Signatrue change of GBeanRegistry.listGBeans [Re: svn commit:
r154941]
Posted by Jeremy Boynes <jb...@apache.org>.
Dain Sundstrom wrote:
> On Feb 23, 2005, at 8:45 AM, Jeremy Boynes wrote:
>
>> Dain Sundstrom wrote:
>>
>>> On Feb 22, 2005, at 7:23 PM, jboynes@apache.org wrote:
>>>
>>>>
>>>> public Set listGBeans(ObjectName pattern) {
>>>> - return gbeanRegistry.listGBeans(pattern);
>>>> + String domain = (pattern == null ||
>>>> pattern.isDomainPattern()) ? null : pattern.getDomain();
>>>> + Map props = pattern == null ? null :
>>>> pattern.getKeyPropertyList();
>>>> + Set gbeans = gbeanRegistry.listGBeans(domain, props);
>>>> + Set result = new HashSet(gbeans.size());
>>>> + for (Iterator i = gbeans.iterator(); i.hasNext();) {
>>>> + GBeanInstance instance = (GBeanInstance) i.next();
>>>> + result.add(instance.getObjectNameObject());
>>>> + }
>>>> + return result;
>>>> }
>>>
>>> On just a cursory review of this commit, it appears that you have
>>> changed the way object name queries work. Before making any other
>>> changes to this code, can you please explain the changes you want to
>>> make, so we can discuss the impact? I am very nervous about any
>>> changes to this code.
>>
>>
>> Perhaps on more detailed examination you'll realize that the only
>> thing changed is that we no longer support partially wildcarded domain
>> patterns, something that we never used and was pretty pointless.
>
>
> So you have removed the feature supporting domain wild cards. I know we
> currently don't use them, but don't see any reason to remove them
> either. Also I belive that you have changed the meaning of an empty
> domain name. In JMX it means match the default domain only, which is
> something I want to begin using in our plans and matching code.
>
Nope, as I said, I removed /partial/ wildcards - stuff like "test?omain"
- full wildcards (i.e. "*" or no value) operate as before. Do you really
see a need to keep that, and if so wouldn't it be better in a more
flexible query mechanism similar to JMX's QueryExp?
I treat a /missing/ domain as a wildcard, which is exactly what we did
before. An empty domain as in ":x=x" is not a wildcard but an exact
match to the domain "". It specifically does not match the default
domain unless that just happens to have been set to "" as well (and the
default default domain is "DefaultDomain").
>
> Anyway, here is my formal -1 roadblock.
>
To what and why?
--
Jeremy
Re: Signatrue change of GBeanRegistry.listGBeans [Re: svn commit: r154941]
Posted by Dain Sundstrom <ds...@gluecode.com>.
On Feb 23, 2005, at 8:45 AM, Jeremy Boynes wrote:
> Dain Sundstrom wrote:
>> On Feb 22, 2005, at 7:23 PM, jboynes@apache.org wrote:
>>>
>>> public Set listGBeans(ObjectName pattern) {
>>> - return gbeanRegistry.listGBeans(pattern);
>>> + String domain = (pattern == null ||
>>> pattern.isDomainPattern()) ? null : pattern.getDomain();
>>> + Map props = pattern == null ? null :
>>> pattern.getKeyPropertyList();
>>> + Set gbeans = gbeanRegistry.listGBeans(domain, props);
>>> + Set result = new HashSet(gbeans.size());
>>> + for (Iterator i = gbeans.iterator(); i.hasNext();) {
>>> + GBeanInstance instance = (GBeanInstance) i.next();
>>> + result.add(instance.getObjectNameObject());
>>> + }
>>> + return result;
>>> }
>> On just a cursory review of this commit, it appears that you have
>> changed the way object name queries work. Before making any other
>> changes to this code, can you please explain the changes you want to
>> make, so we can discuss the impact? I am very nervous about any
>> changes to this code.
>
> Perhaps on more detailed examination you'll realize that the only
> thing changed is that we no longer support partially wildcarded domain
> patterns, something that we never used and was pretty pointless.
So you have removed the feature supporting domain wild cards. I know
we currently don't use them, but don't see any reason to remove them
either. Also I belive that you have changed the meaning of an empty
domain name. In JMX it means match the default domain only, which is
something I want to begin using in our plans and matching code.
> Also, given we're changing the API anyway, there is the opportunity
> here to remove the stupid overloading of names and patterns and have a
> much clearer query API.
I don't agree on spliting names and patterns. There is a natural
connection between patterns and specific names. In the reference code,
it is nice to say, I want a reference to this exact object using a non
pattern object name. The interchangeability between specific and
pattern names is very convenient and I don't see any reason to remove
the feature.
> We have talked about the changes here extensively and all agreed on
> the direction. Continue to be nervous, but please don't raise
> roadblocks without due consideration.
I don't remember discussing discussing removing domain queries, or
splitting object names and patters. Maybe you can refresh my memeory.
Anyway, here is my formal -1 roadblock.
-dain
Re: Signatrue change of GBeanRegistry.listGBeans [Re: svn commit:
r154941]
Posted by Jeremy Boynes <jb...@apache.org>.
Dain Sundstrom wrote:
> On Feb 22, 2005, at 7:23 PM, jboynes@apache.org wrote:
>
>>
>> public Set listGBeans(ObjectName pattern) {
>> - return gbeanRegistry.listGBeans(pattern);
>> + String domain = (pattern == null ||
>> pattern.isDomainPattern()) ? null : pattern.getDomain();
>> + Map props = pattern == null ? null :
>> pattern.getKeyPropertyList();
>> + Set gbeans = gbeanRegistry.listGBeans(domain, props);
>> + Set result = new HashSet(gbeans.size());
>> + for (Iterator i = gbeans.iterator(); i.hasNext();) {
>> + GBeanInstance instance = (GBeanInstance) i.next();
>> + result.add(instance.getObjectNameObject());
>> + }
>> + return result;
>> }
>
>
> On just a cursory review of this commit, it appears that you have
> changed the way object name queries work. Before making any other
> changes to this code, can you please explain the changes you want to
> make, so we can discuss the impact? I am very nervous about any changes
> to this code.
>
Perhaps on more detailed examination you'll realize that the only thing
changed is that we no longer support partially wildcarded domain
patterns, something that we never used and was pretty pointless.
Also, given we're changing the API anyway, there is the opportunity here
to remove the stupid overloading of names and patterns and have a much
clearer query API.
We have talked about the changes here extensively and all agreed on the
direction. Continue to be nervous, but please don't raise roadblocks
without due consideration.
--
Jeremy
Signatrue change of GBeanRegistry.listGBeans [Re: svn commit: r154941]
Posted by Dain Sundstrom <ds...@gluecode.com>.
On Feb 22, 2005, at 7:23 PM, jboynes@apache.org wrote:
>
> public Set listGBeans(ObjectName pattern) {
> - return gbeanRegistry.listGBeans(pattern);
> + String domain = (pattern == null ||
> pattern.isDomainPattern()) ? null : pattern.getDomain();
> + Map props = pattern == null ? null :
> pattern.getKeyPropertyList();
> + Set gbeans = gbeanRegistry.listGBeans(domain, props);
> + Set result = new HashSet(gbeans.size());
> + for (Iterator i = gbeans.iterator(); i.hasNext();) {
> + GBeanInstance instance = (GBeanInstance) i.next();
> + result.add(instance.getObjectNameObject());
> + }
> + return result;
> }
On just a cursory review of this commit, it appears that you have
changed the way object name queries work. Before making any other
changes to this code, can you please explain the changes you want to
make, so we can discuss the impact? I am very nervous about any
changes to this code.
-dain