ISPN-2492 Rename the XSiteAdminOperations.status(String) method to si…

…teStatus(String) to avoid overloading which

breaks RHQ
Add additional checks in the RHQ plugin metadata generation to avoid accidental duplicates

core/src/main/java/org/infinispan/interceptors/CacheLoaderInterceptor.java

@@ -273,7 +273,7 @@ public Collection<String> getCacheLoaders() {
       }
    }
    @ManagedOperation(description = "Disable all cache loaders of a given type, where type is a fully qualified class name of the cache loader to disable")
-   @Operation(displayName = "Disable all cache loaders of a given type, where type is a fully qualified class name of the cache loader to disable")
+   @Operation(displayName = "Disable all cache loaders of a given type")
    /**
     * Disables a cache loader of a given type, where type is the fully qualified class name of a {@link CacheLoader} implementation.
     *

core/src/main/java/org/infinispan/upgrade/RollingUpgradeManager.java

@@ -41,7 +41,7 @@
 public class RollingUpgradeManager {
    private final ConcurrentMap<String, Migrator> migrators = new ConcurrentHashMap<String, Migrator>(2);
    @ManagedOperation(description = "Dumps the global known keyset to a well-known key for retrieval by the upgrade process")
-   @Operation(displayName = "Dumps the global known keyset to a well-known key for retrieval by the upgrade process")
+   @Operation(displayName = "Dumps the global known keyset")
    public void recordKnownGlobalKeyset() {
       for (Migrator m: migrators.values()) m.recordKnownGlobalKeyset();
    }

core/src/main/java/org/infinispan/xsite/XSiteAdminOperations.java

@@ -66,7 +66,7 @@ public void init(RpcManager rpcManager, BackupSender backupSender, Cache cache)
 
    @Operation(displayName = "Check whether the given backup site is offline or not.")
    @ManagedOperation(description = "Check whether the given backup site is offline or not.")
-   public String status(String site) {
+   public String siteStatus(String site) {
       //also consider local node
       OfflineStatus offlineStatus = backupSender.getOfflineStatus(site);
       if (offlineStatus == null)
@@ -185,13 +185,13 @@ public String takeSiteOffline(String site) {
       return returnFailureOrSuccess(failed, prefix);
    }
 
-   @Operation(displayName = "Amends the values for 'afterFailures' for the 'TakeOffline' functionality on all the nodes in the cluster.")
+   @Operation(displayName = "Amends the values for 'TakeOffline.afterFailures' on all the nodes in the cluster.")
    @ManagedOperation(description = "Amends the values for 'afterFailures' for the 'TakeOffline' functionality on all the nodes in the cluster.")
    public String setTakeOfflineAfterFailures(String site, int afterFailures) {
       return takeOffline(site, afterFailures, null);
    }
 
-   @Operation(displayName = "Amends the values for 'minTimeToWait' for the 'TakeOffline' functionality on all the nodes in the cluster.")
+   @Operation(displayName = "Amends the values for 'TakeOffline.minTimeToWait' on all the nodes in the cluster.")
    @ManagedOperation(description = "Amends the values for 'minTimeToWait' for the 'TakeOffline' functionality on all the nodes in the cluster.")
    public String setTakeOfflineMinTimeToWait(String site, long minTimeToWait) {
       return takeOffline(site, null, minTimeToWait);

core/src/test/java/org/infinispan/xsite/XSiteAdminOperationsTest.java

@@ -48,22 +48,22 @@ protected ConfigurationBuilder getLonActiveConfig() {
    }
 
    public void testSiteStatus() {
-      assertEquals(admin("LON", 0).status("NYC"), XSiteAdminOperations.ONLINE);
-      assertEquals(admin("LON", 1).status("NYC"), XSiteAdminOperations.ONLINE);
+      assertEquals(admin("LON", 0).siteStatus("NYC"), XSiteAdminOperations.ONLINE);
+      assertEquals(admin("LON", 1).siteStatus("NYC"), XSiteAdminOperations.ONLINE);
 
       assertEquals(XSiteAdminOperations.SUCCESS, admin("LON", 1).takeSiteOffline("NYC"));
 
-      assertEquals(admin("LON", 0).status("NYC"), XSiteAdminOperations.OFFLINE);
-      assertEquals(admin("LON", 1).status("NYC"), XSiteAdminOperations.OFFLINE);
+      assertEquals(admin("LON", 0).siteStatus("NYC"), XSiteAdminOperations.OFFLINE);
+      assertEquals(admin("LON", 1).siteStatus("NYC"), XSiteAdminOperations.OFFLINE);
 
       assertEquals(XSiteAdminOperations.SUCCESS, admin("LON", 1).bringSiteOnline("NYC"));
-      assertEquals(admin("LON", 0).status("NYC"), XSiteAdminOperations.ONLINE);
-      assertEquals(admin("LON", 1).status("NYC"), XSiteAdminOperations.ONLINE);
+      assertEquals(admin("LON", 0).siteStatus("NYC"), XSiteAdminOperations.ONLINE);
+      assertEquals(admin("LON", 1).siteStatus("NYC"), XSiteAdminOperations.ONLINE);
    }
 
    public void amendTakeOffline() {
-      assertEquals(admin("LON", 0).status("NYC"), XSiteAdminOperations.ONLINE);
-      assertEquals(admin("LON", 1).status("NYC"), XSiteAdminOperations.ONLINE);
+      assertEquals(admin("LON", 0).siteStatus("NYC"), XSiteAdminOperations.ONLINE);
+      assertEquals(admin("LON", 1).siteStatus("NYC"), XSiteAdminOperations.ONLINE);
 
       BackupSenderImpl bs = backupSender("LON", 0);
       OfflineStatus offlineStatus = bs.getOfflineStatus("NYC");

tools/src/main/java/org/infinispan/tools/rhq/RhqPluginXmlGenerator.java

@@ -183,6 +183,7 @@ private static void populateMetricsAndOperations(List<Class<?>> classes,
             Props props, boolean withNamePrefix) throws Exception {
       props.setHasOperations(true);
       props.setHasMetrics(true);
+      Set<String> uniqueOperations = new HashSet<String>();
       for (Class<?> clazz : classes) {
          MBean mbean = clazz.getAnnotation(MBean.class);
          String prefix = withNamePrefix ? mbean.objectName() + '.' : "";
@@ -207,6 +208,7 @@ private static void populateMetricsAndOperations(List<Class<?>> classes,
                }
                MetricProps metric = new MetricProps(property);
                String displayName = withNamePrefix ? "[" + mbean.objectName() + "] " + rhqMetric.displayName() : rhqMetric.displayName();
+               validateDisplayName(displayName);
                metric.setDisplayName(displayName);
                metric.setDisplayType(rhqMetric.displayType());
                metric.setDataType(rhqMetric.dataType());
@@ -233,8 +235,13 @@ private static void populateMetricsAndOperations(List<Class<?>> classes,
                } else {
                   name = prefix + ctMethod.getName();
                }
+               if (uniqueOperations.contains(name)) {
+                  throw new RuntimeException("Duplicate operation name: "+name);
+               }
+               uniqueOperations.add(name);
                OperationProps operation = new OperationProps(name);
                String displayName = withNamePrefix ? "[" + mbean.objectName() + "] " + rhqOperation.displayName() : rhqOperation.displayName();
+               validateDisplayName(displayName);
                operation.setDisplayName(displayName);
                if (managedAttr != null) {
                   debug("Operation has ManagedAttribute annotation " + managedAttr);
@@ -291,6 +298,7 @@ private static void populateMetricsAndOperations(List<Class<?>> classes,
                }
                MetricProps metric = new MetricProps(property);
                String displayName = withNamePrefix ? "[" + mbean.objectName() + "] " + rhqMetric.displayName() : rhqMetric.displayName();
+               validateDisplayName(displayName);
                metric.setDisplayName(displayName);
                metric.setDisplayType(rhqMetric.displayType());
                metric.setDataType(rhqMetric.dataType());
@@ -311,6 +319,12 @@ private static void populateMetricsAndOperations(List<Class<?>> classes,
       }
    }
 
+   private static void validateDisplayName(String displayName) {
+      if (displayName.length() > 100) {
+         throw new RuntimeException("Display name too long (max 100 chars): "+displayName);
+      }
+   }
+
    private static String getPropertyFromBeanConvention(CtMethod ctMethod) {
       String getterOrSetter = ctMethod.getName();
       if (getterOrSetter.startsWith("get") || getterOrSetter.startsWith("set")) {