6731410: JMXServiceURL cannot use @ConstructorProperties for compatibility reasons

Reviewed-by: dfuchs

src/share/classes/javax/management/MXBean.java

@@ -469,8 +469,8 @@ public class MemoryPool
       <em>J</em>, then <em>J</em> cannot be the type of a method
       parameter or return value in an MXBean interface.</p>
 
-    <p>If there is a way to convert <em>opendata(J)</em> back to
-      <em>J</em> then we say that <em>J</em> is
+    <p id="reconstructible-def">If there is a way to convert
+      <em>opendata(J)</em> back to <em>J</em> then we say that <em>J</em> is
       <em>reconstructible</em>.  All method parameters in an MXBean
       interface must be reconstructible, because when the MXBean
       framework is invoking a method it will need to convert those

src/share/classes/javax/management/remote/JMXServiceURL.java

@@ -30,13 +30,14 @@ package javax.management.remote;
 import com.sun.jmx.remote.util.ClassLogger;
 import com.sun.jmx.remote.util.EnvHelp;
 
-import java.beans.ConstructorProperties;
 import java.io.Serializable;
 import java.net.InetAddress;
 import java.net.MalformedURLException;
 import java.net.UnknownHostException;
 import java.util.BitSet;
 import java.util.StringTokenizer;
+import javax.management.openmbean.CompositeData;
+import javax.management.openmbean.InvalidKeyException;
 
 /**
  * <p>The address of a JMX API connector server.  Instances of this class
@@ -273,7 +274,6 @@ public class JMXServiceURL implements Serializable {
      * is not possible to find the local host name, or if
      * <code>port</code> is negative.
      */
-    @ConstructorProperties({"protocol", "host", "port", "URLPath"})
     public JMXServiceURL(String protocol, String host, int port,
                          String urlPath)
             throws MalformedURLException {
@@ -338,6 +338,50 @@ public class JMXServiceURL implements Serializable {
         validate();
     }
 
+    /**
+     * <p>Construct a {@code JMXServiceURL} instance from the given
+     * {@code CompositeData}.  The presence of this method means that
+     * {@code JMXServiceURL} is <a href="../MXBean.html#reconstructible-def">
+     * reconstructible</a> in MXBeans.</p>
+     *
+     * <p>(The effect of this method could have been obtained more simply
+     * with a &#64;{@link java.beans.ConstructorProperties ConstructorProperties}
+     * annotation on the four-parameter {@linkplain #JMXServiceURL(
+     * String, String, int, String) constructor}, but that would have meant
+     * that this API could not be implemented on versions of the Java platform
+     * that predated the introduction of that annotation.)</p>
+     *
+     * @param cd a {@code CompositeData} object that must contain items called
+     * {@code protocol}, {@code host}, and {@code URLPath} of type {@code String}
+     * and {@code port} of type {@code Integer}.  Such an object will be produced
+     * by the MXBean framework when <a
+     * href="../MXBean.html#composite-map">mapping</a> a {@code JMXServiceURL}
+     * instance to an Open Data value.
+     *
+     * @return a {@code JMXServiceURL} constructed with the protocol, host,
+     * port, and URL path extracted from the given {@code CompositeData}.
+     *
+     * @throws MalformedURLException if the given {@code CompositeData} does
+     * not contain all the required items with the required types or if the
+     * resultant URL is syntactically incorrect.
+     */
+    public static JMXServiceURL from(CompositeData cd)
+            throws MalformedURLException {
+        try {
+            String proto = (String) cd.get("protocol");
+            String host = (String) cd.get("host");
+            int port = (Integer) cd.get("port");
+            String urlPath = (String) cd.get("URLPath");
+            return new JMXServiceURL(proto, host, port, urlPath);
+        } catch (RuntimeException e) {
+            // Could be InvalidKeyException if the item is missing,
+            // or ClassCastException if it is present but with the wrong type.
+            MalformedURLException x = new MalformedURLException(e.getMessage());
+            x.initCause(e);
+            throw x;
+        }
+    }
+
     private void validate() throws MalformedURLException {
 
         // Check protocol

test/javax/management/mxbean/JMXServiceURLTest.java

@@ -23,16 +23,24 @@
 
 /*
  * @test JMXServiceURLTest
- * @bug 6607114 6670375
+ * @bug 6607114 6670375 6731410
  * @summary Test that JMXServiceURL works correctly in MXBeans
  * @author Eamonn McManus
  */
 
+import java.io.InvalidObjectException;
 import java.lang.management.ManagementFactory;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.management.Attribute;
 import javax.management.JMX;
+import javax.management.MBeanException;
 import javax.management.MBeanServer;
 import javax.management.ObjectName;
 import javax.management.openmbean.CompositeData;
+import javax.management.openmbean.CompositeDataSupport;
 import javax.management.openmbean.CompositeType;
 import javax.management.openmbean.OpenType;
 import javax.management.openmbean.SimpleType;
@@ -56,6 +64,25 @@ public class JMXServiceURLTest {
         }
     }
 
+    private static enum Part {
+        PROTOCOL("protocol", SimpleType.STRING, "rmi", 25, "", "a:b", "/", "?", "#"),
+        HOST("host", SimpleType.STRING, "a.b.c", 25, "a..b", ".a.b", "a.b."),
+        PORT("port", SimpleType.INTEGER, 25, "25", -25),
+        PATH("URLPath", SimpleType.STRING, "/tiddly", 25, "tiddly");
+
+        Part(String name, OpenType openType, Object validValue, Object... bogusValues) {
+            this.name = name;
+            this.openType = openType;
+            this.validValue = validValue;
+            this.bogusValues = bogusValues;
+        }
+
+        final String name;
+        final OpenType openType;
+        final Object validValue;
+        final Object[] bogusValues;
+    }
+
     public static void main(String[] args) throws Exception {
         MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
         ObjectName name = new ObjectName("a:b=c");
@@ -91,6 +118,92 @@ public class JMXServiceURLTest {
             Object actualValue = cd.get(itemName);
             assertEquals(expectedValue, actualValue);
         }
+
+        // Now make sure we reject any bogus-looking CompositeData items.
+        // We first try every combination of omitted items (items can be
+        // null but cannot be omitted), then we try every combination of
+        // valid and bogus items.
+        final Part[] parts = Part.values();
+        final int nParts = parts.length;
+        final int maxPartMask = (1 << nParts) - 1;
+        // Iterate over all possibilities of included and omitted, except
+        // 0, because a CompositeDataSupport must have at least one element,
+        // and maxPartMask, where all items are included and the result is valid.
+        for (int mask = 1; mask < maxPartMask; mask++) {
+            Map<String, Object> cdMap = new HashMap<String, Object>();
+            List<String> names = new ArrayList<String>();
+            List<OpenType> types = new ArrayList<OpenType>();
+            for (int i = 0; i < nParts; i++) {
+                if ((mask & (1 << i)) != 0) {
+                    Part part = parts[i];
+                    cdMap.put(part.name, part.validValue);
+                    names.add(part.name);
+                    types.add(openTypeForValue(part.validValue));
+                }
+            }
+            String[] nameArray = names.toArray(new String[0]);
+            OpenType[] typeArray = types.toArray(new OpenType[0]);
+            CompositeType badct = new CompositeType(
+                    "bad", "descr", nameArray, nameArray, typeArray);
+            CompositeData badcd = new CompositeDataSupport(badct, cdMap);
+            checkBad(mbs, name, badcd);
+        }
+
+        int nBogus = 1;
+        for (Part part : parts)
+            nBogus *= (part.bogusValues.length + 1);
+        // Iterate over all combinations of bogus values.  We are basically
+        // treating each Part as a digit while counting up from 1.  A digit
+        // value of 0 stands for the valid value of that Part, and 1 on
+        // stand for the bogus values.  Hence an integer where all the digits
+        // are 0 would represent a valid CompositeData, which is why we
+        // start from 1.
+        for (int bogusCount = 1; bogusCount < nBogus; bogusCount++) {
+            List<String> names = new ArrayList<String>();
+            List<OpenType> types = new ArrayList<OpenType>();
+            int x = bogusCount;
+            Map<String, Object> cdMap = new HashMap<String, Object>();
+            for (Part part : parts) {
+                int digitMax = part.bogusValues.length + 1;
+                int digit = x % digitMax;
+                Object value = (digit == 0) ?
+                    part.validValue : part.bogusValues[digit - 1];
+                cdMap.put(part.name, value);
+                names.add(part.name);
+                types.add(openTypeForValue(value));
+                x /= digitMax;
+            }
+            String[] nameArray = names.toArray(new String[0]);
+            OpenType[] typeArray = types.toArray(new OpenType[0]);
+            CompositeType badct = new CompositeType(
+                    "bad", "descr", nameArray, nameArray, typeArray);
+            CompositeData badcd = new CompositeDataSupport(badct, cdMap);
+            checkBad(mbs, name, badcd);
+        }
+    }
+
+    private static OpenType openTypeForValue(Object value) {
+        if (value instanceof String)
+            return SimpleType.STRING;
+        else if (value instanceof Integer)
+            return SimpleType.INTEGER;
+        else
+            throw new AssertionError("Value has invalid type: " + value);
+    }
+
+    private static void checkBad(
+            MBeanServer mbs, ObjectName name, CompositeData badcd)
+            throws Exception {
+        try {
+            mbs.setAttribute(name, new Attribute("Url", badcd));
+            throw new Exception("Expected exception for: " + badcd);
+        } catch (MBeanException e) {
+            if (!(e.getCause() instanceof InvalidObjectException)) {
+                throw new Exception(
+                        "Wrapped exception should be InvalidObjectException", e);
+            }
+            System.out.println("OK: rejected " + badcd);
+        }
     }
 
     private static void assertEquals(Object expect, Object actual)