You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2020/06/11 04:40:50 UTC

[GitHub] [guacamole-client] mike-jumper opened a new pull request #519: GUACAMOLE-708: Implement EnumGuacamoleProperty.

mike-jumper opened a new pull request #519:
URL: https://github.com/apache/guacamole-client/pull/519


   This change implements a new property type, `EnumGuacamoleProperty`, intended to ease creation of properties that are driven by `enum` constants. Such properties formerly required defining a custom `GuacamoleProperty` subclass.
   
   `EnumGuacamoleProperty` allows string/enum mapping through either of the following:
   
   * Annotating the constants of an enum with their corresponding string values using `@PropertyValue`.
   * Manually listing the constants and corresponding strings of an enum (typically necessary if the definition of the enum cannot be modified).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] necouchman commented on a change in pull request #519: GUACAMOLE-728: Implement EnumGuacamoleProperty.

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #519:
URL: https://github.com/apache/guacamole-client/pull/519#discussion_r439776433



##########
File path: extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/RadiusAuthenticationProtocol.java
##########
@@ -19,100 +19,54 @@
 
 package org.apache.guacamole.auth.radius.conf;
 
+import org.apache.guacamole.properties.EnumGuacamoleProperty.PropertyValue;
+
 /**
  * This enum represents supported RADIUS authentication protocols for
  * the guacamole-auth-radius extension.
  */
 public enum RadiusAuthenticationProtocol {
     
     /**
-     * Password Authentication Protocol (PAP)
+     * Password Authentication Protocol (PAP).
      */
-    PAP("pap"),
+    @PropertyValue("pap")
+    PAP,
     
     /**
-     * Challenge-Handshake Authentication Protocol (CHAP)
+     * Challenge-Handshake Authentication Protocol (CHAP).
      */
-    CHAP("chap"),
+    @PropertyValue("chap")
+    CHAP,
     
     /**
-     * Microsoft implementation of CHAP, Version 1 (MS-CHAPv1)
+     * Microsoft implementation of CHAP, Version 1 (MS-CHAPv1).
      */
-    MSCHAPv1("mschapv1"),
+    @PropertyValue("mschapv1")
+    MSCHAP_V1,
     
     /**
-     * Microsoft implementation of CHAP, Version 2 (MS-CHAPv2)
+     * Microsoft implementation of CHAP, Version 2 (MS-CHAPv2).
      */
-    MSCHAPv2("mschapv2"),
+    @PropertyValue("mschapv2")
+    MSCHAP_V2,
     
     /**
-     * Extensible Authentication Protocol (EAP) with MD5 Hashing (EAP-MD5)
+     * Extensible Authentication Protocol (EAP) with MD5 Hashing (EAP-MD5).
      */
-    EAP_MD5("eap-md5"),
+    @PropertyValue("eap-md5")
+    EAP_MD5,
 
     /**
      * Extensible Authentication Protocol (EAP) with TLS encryption (EAP-TLS).
      */
-    EAP_TLS("eap-tls"),
+    @PropertyValue("eap-tls")
+    EAP_TLS,
 
     /**
      * Extensible Authentication Protocol (EAP) with Tunneled TLS (EAP-TTLS).
      */
-    EAP_TTLS("eap-ttls");
+    @PropertyValue("eap-ttls")
+    EAP_TTLS;
 
-    /**
-     * This variable stores the string value of the protocol, and is also
-     * used within the extension to pass to JRadius for configuring the
-     * library to talk to the RADIUS server.
-     */
-    private final String strValue;
-    
-    /**
-     * Create a new RadiusAuthenticationProtocol object having the
-     * given string value.
-     * 
-     * @param strValue
-     *     The value of the protocol to store as a string, which will be used
-     *     in specifying the protocol within the guacamole.properties file, and
-     *     will also be used by the JRadius library for its configuration.

Review comment:
       Okay, so, as long as it properly converts to a `String` using `toString()`, it should be okay:
   ```
           RadiusAuthenticator radAuth = radiusClient.getAuthProtocol(
                   confService.getRadiusAuthProtocol().toString());
           
           if (radAuth == null)
               throw new GuacamoleException("Could not get a valid RadiusAuthenticator for specified protocol: " + confService.getRadiusAuthProtocol());
   ```
   (https://github.com/apache/guacamole-client/blob/4e4b6f24f2d34a91f5381da955680f85c2ce56c3/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusConnectionService.java#L129-L133)
   
   The Inner Protocol may have to be re-worked, a bit:
   
   ```
           // If we're using EAP-TTLS, we need to define tunneled protocol
           if (radAuth instanceof EAPTTLSAuthenticator) {
               RadiusAuthenticationProtocol innerProtocol =
                       confService.getRadiusEAPTTLSInnerProtocol();
               
               if (innerProtocol == null)
                   throw new GuacamoleException("Missing or invalid inner protocol for EAP-TTLS.");
   
   
               ((EAPTTLSAuthenticator)radAuth).setInnerProtocol(innerProtocol.toString());
   ```
   (https://github.com/apache/guacamole-client/blob/4e4b6f24f2d34a91f5381da955680f85c2ce56c3/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusConnectionService.java#L161-L170)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #519: GUACAMOLE-728: Implement EnumGuacamoleProperty.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #519:
URL: https://github.com/apache/guacamole-client/pull/519#discussion_r439776541



##########
File path: guacamole-ext/src/test/java/org/apache/guacamole/properties/EnumGuacamolePropertyTest.java
##########
@@ -0,0 +1,372 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.properties;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.properties.EnumGuacamoleProperty.PropertyValue;
+import static org.junit.Assert.*;
+import org.junit.Test;
+
+/**
+ * Test which verifies that EnumGuacamoleProperty functions correctly.
+ */
+public class EnumGuacamolePropertyTest {
+
+    /**
+     * Example enum consisting of a small set of possible fish. All values of
+     * this enum are annotated with {@link PropertyValue}.
+     */
+    public static enum Fish {
+
+        /**
+         * Salmon are large, anadromous fish prized for their pink/red/orange
+         * flesh.
+         *
+         * @see <a href="https://en.wikipedia.org/wiki/Salmon">Salmon (Wikipedia)</a>
+         */
+        @PropertyValue("salmon")
+        SALMON,
+
+        /**
+         * Trout are freshwater fish related to salmon, popular both as food
+         * and as game fish.
+         *
+         * @see <a href="https://en.wikipedia.org/wiki/Trout">Trout (Wikipedia)</a>
+         */
+        @PropertyValue("trout")
+        TROUT,
+
+        /**
+         * Mackerel are pelagic fish, typically having vertical stripes along
+         * their backs.
+         *
+         * @see <a href="https://en.wikipedia.org/wiki/Mackerel">Mackerel (Wikipedia)</a>
+         */
+        @PropertyValue("mackerel")
+        MACKEREL,
+
+        /**
+         * Tuna are large, predatory, saltwater fish in the same family as
+         * mackerel. They are one of the few fish that can maintain a body
+         * temperature higher than the surrounding water.
+         *
+         * @see <a href="https://en.wikipedia.org/wiki/Tuna">Tuna (Wikipedia)</a>
+         */
+        @PropertyValue("tuna")
+        TUNA,
+
+        /**
+         * Sardines are small, herring-like fish commonly served in cans.
+         * Sardines are considered prey fish and feed almost exclusively on
+         * zooplankton.
+         *
+         * @see <a href="https://en.wikipedia.org/wiki/Sardine">Sardine (Wikipedia)</a>
+         */
+        @PropertyValue("sardine")
+        SARDINE
+
+    }
+
+    /**
+     * Example enum consisting of a small set of possible vegetables. None of
+     * the values of this enum are annotated with {@link PropertyValue}.
+     */
+    public static enum Vegetable {
+
+        /**
+         * Potatoes are starchy root vegetables native to the Americas. The
+         * tuber itself is edible, but other parts can be toxic.
+         *
+         * @see <a href="https://en.wikipedia.org/wiki/Potato">Potato (Wikipedia)</a>
+         */
+        POTATO,
+
+        /**
+         * Carrots are root vegetables, tapered in shape and generally orange
+         * in color.
+         *
+         * @see <a href="https://en.wikipedia.org/wiki/Carrot">Carrot (Wikipedia)</a>
+         */
+        CARROT
+
+    }
+
+    /**
+     * Example Guacamole property which parses String values as Fish constants.
+     */
+    private static final EnumGuacamoleProperty<Fish> FAVORITE_FISH = new EnumGuacamoleProperty<Fish>(Fish.class) {
+
+        @Override
+        public String getName() {
+            return "favorite-fish";
+        }
+
+    };
+
+    /**
+     * Verifies that EnumGuacamoleProperty correctly parses string values that
+     * are associated with their corresponding enum constants using the
+     * {@link PropertyValue} annotation.
+     *
+     * @throws GuacamoleException
+     *     If a valid test value is incorrectly recognized by parseValue() as
+     *     invalid.
+     */
+    @Test
+    public void testParseValue() throws GuacamoleException {
+        assertEquals(Fish.SALMON,   FAVORITE_FISH.parseValue("salmon"));
+        assertEquals(Fish.TROUT,    FAVORITE_FISH.parseValue("trout"));
+        assertEquals(Fish.MACKEREL, FAVORITE_FISH.parseValue("mackerel"));
+        assertEquals(Fish.TUNA,     FAVORITE_FISH.parseValue("tuna"));
+        assertEquals(Fish.SARDINE,  FAVORITE_FISH.parseValue("sardine"));
+    }
+
+    /**
+     * Verifies that the absence of a property value (null) is parsed by
+     * EnumGuacamoleProperty as the absence of an enum constant (also null).
+     *
+     * @throws GuacamoleException
+     *     If a valid test value is incorrectly recognized by parseValue() as
+     *     invalid.
+     */
+    @Test
+    public void testParseNullValue() throws GuacamoleException {
+        assertNull(FAVORITE_FISH.parseValue(null));
+    }
+
+    /**
+     * Verifies that GuacamoleException is thrown when attempting to parse an
+     * invalid value, and that the error message contains a sorted list of all
+     * allowed values.
+     */
+    @Test
+    public void testParseInvalidValue() {
+        try {
+            FAVORITE_FISH.parseValue("anchovy");
+            fail("Invalid EnumGuacamoleProperty values should fail to parse with an exception.");

Review comment:
       True.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #519: GUACAMOLE-728: Implement EnumGuacamoleProperty.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #519:
URL: https://github.com/apache/guacamole-client/pull/519#discussion_r439778480



##########
File path: extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/RadiusAuthenticationProtocol.java
##########
@@ -19,100 +19,54 @@
 
 package org.apache.guacamole.auth.radius.conf;
 
+import org.apache.guacamole.properties.EnumGuacamoleProperty.PropertyValue;
+
 /**
  * This enum represents supported RADIUS authentication protocols for
  * the guacamole-auth-radius extension.
  */
 public enum RadiusAuthenticationProtocol {
     
     /**
-     * Password Authentication Protocol (PAP)
+     * Password Authentication Protocol (PAP).
      */
-    PAP("pap"),
+    @PropertyValue("pap")
+    PAP,
     
     /**
-     * Challenge-Handshake Authentication Protocol (CHAP)
+     * Challenge-Handshake Authentication Protocol (CHAP).
      */
-    CHAP("chap"),
+    @PropertyValue("chap")
+    CHAP,
     
     /**
-     * Microsoft implementation of CHAP, Version 1 (MS-CHAPv1)
+     * Microsoft implementation of CHAP, Version 1 (MS-CHAPv1).
      */
-    MSCHAPv1("mschapv1"),
+    @PropertyValue("mschapv1")
+    MSCHAP_V1,
     
     /**
-     * Microsoft implementation of CHAP, Version 2 (MS-CHAPv2)
+     * Microsoft implementation of CHAP, Version 2 (MS-CHAPv2).
      */
-    MSCHAPv2("mschapv2"),
+    @PropertyValue("mschapv2")
+    MSCHAP_V2,
     
     /**
-     * Extensible Authentication Protocol (EAP) with MD5 Hashing (EAP-MD5)
+     * Extensible Authentication Protocol (EAP) with MD5 Hashing (EAP-MD5).
      */
-    EAP_MD5("eap-md5"),
+    @PropertyValue("eap-md5")
+    EAP_MD5,
 
     /**
      * Extensible Authentication Protocol (EAP) with TLS encryption (EAP-TLS).
      */
-    EAP_TLS("eap-tls"),
+    @PropertyValue("eap-tls")
+    EAP_TLS,
 
     /**
      * Extensible Authentication Protocol (EAP) with Tunneled TLS (EAP-TTLS).
      */
-    EAP_TTLS("eap-ttls");
+    @PropertyValue("eap-ttls")
+    EAP_TTLS;
 
-    /**
-     * This variable stores the string value of the protocol, and is also
-     * used within the extension to pass to JRadius for configuring the
-     * library to talk to the RADIUS server.
-     */
-    private final String strValue;
-    
-    /**
-     * Create a new RadiusAuthenticationProtocol object having the
-     * given string value.
-     * 
-     * @param strValue
-     *     The value of the protocol to store as a string, which will be used
-     *     in specifying the protocol within the guacamole.properties file, and
-     *     will also be used by the JRadius library for its configuration.

Review comment:
       OK - I've added:
   
   * A constructor parameter for the JRadius protocol name (referencing the specific `NAME` constant) and static property, `JRADIUS_PROTOCOL_NAME`, that exposes this.
   * A convenience function, `getAuthenticator()`, which returns the `RadiusAuthenticator` instance for the protocol in question.
   
   and carried that refactor through.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] necouchman commented on a change in pull request #519: GUACAMOLE-728: Implement EnumGuacamoleProperty.

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #519:
URL: https://github.com/apache/guacamole-client/pull/519#discussion_r439776433



##########
File path: extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/RadiusAuthenticationProtocol.java
##########
@@ -19,100 +19,54 @@
 
 package org.apache.guacamole.auth.radius.conf;
 
+import org.apache.guacamole.properties.EnumGuacamoleProperty.PropertyValue;
+
 /**
  * This enum represents supported RADIUS authentication protocols for
  * the guacamole-auth-radius extension.
  */
 public enum RadiusAuthenticationProtocol {
     
     /**
-     * Password Authentication Protocol (PAP)
+     * Password Authentication Protocol (PAP).
      */
-    PAP("pap"),
+    @PropertyValue("pap")
+    PAP,
     
     /**
-     * Challenge-Handshake Authentication Protocol (CHAP)
+     * Challenge-Handshake Authentication Protocol (CHAP).
      */
-    CHAP("chap"),
+    @PropertyValue("chap")
+    CHAP,
     
     /**
-     * Microsoft implementation of CHAP, Version 1 (MS-CHAPv1)
+     * Microsoft implementation of CHAP, Version 1 (MS-CHAPv1).
      */
-    MSCHAPv1("mschapv1"),
+    @PropertyValue("mschapv1")
+    MSCHAP_V1,
     
     /**
-     * Microsoft implementation of CHAP, Version 2 (MS-CHAPv2)
+     * Microsoft implementation of CHAP, Version 2 (MS-CHAPv2).
      */
-    MSCHAPv2("mschapv2"),
+    @PropertyValue("mschapv2")
+    MSCHAP_V2,
     
     /**
-     * Extensible Authentication Protocol (EAP) with MD5 Hashing (EAP-MD5)
+     * Extensible Authentication Protocol (EAP) with MD5 Hashing (EAP-MD5).
      */
-    EAP_MD5("eap-md5"),
+    @PropertyValue("eap-md5")
+    EAP_MD5,
 
     /**
      * Extensible Authentication Protocol (EAP) with TLS encryption (EAP-TLS).
      */
-    EAP_TLS("eap-tls"),
+    @PropertyValue("eap-tls")
+    EAP_TLS,
 
     /**
      * Extensible Authentication Protocol (EAP) with Tunneled TLS (EAP-TTLS).
      */
-    EAP_TTLS("eap-ttls");
+    @PropertyValue("eap-ttls")
+    EAP_TTLS;
 
-    /**
-     * This variable stores the string value of the protocol, and is also
-     * used within the extension to pass to JRadius for configuring the
-     * library to talk to the RADIUS server.
-     */
-    private final String strValue;
-    
-    /**
-     * Create a new RadiusAuthenticationProtocol object having the
-     * given string value.
-     * 
-     * @param strValue
-     *     The value of the protocol to store as a string, which will be used
-     *     in specifying the protocol within the guacamole.properties file, and
-     *     will also be used by the JRadius library for its configuration.

Review comment:
       Okay, so, as long as it properly converts to a `String` using `toString()`, it should be okay:
   https://github.com/apache/guacamole-client/blob/4e4b6f24f2d34a91f5381da955680f85c2ce56c3/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusConnectionService.java#L129-L133
   
   The Inner Protocol may have to be re-worked, a bit:
   https://github.com/apache/guacamole-client/blob/4e4b6f24f2d34a91f5381da955680f85c2ce56c3/extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusConnectionService.java#L161-L170




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] necouchman commented on a change in pull request #519: GUACAMOLE-728: Implement EnumGuacamoleProperty.

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #519:
URL: https://github.com/apache/guacamole-client/pull/519#discussion_r439706084



##########
File path: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/conf/LDAPGuacamoleProperties.java
##########
@@ -181,8 +183,13 @@ private LDAPGuacamoleProperties() {}
      * Property that controls whether or not the LDAP connection follows
      * (dereferences) aliases as it searches the tree.
      */
-    public static final DereferenceAliasesProperty LDAP_DEREFERENCE_ALIASES =
-            new DereferenceAliasesProperty() {
+    public static final EnumGuacamoleProperty<AliasDerefMode> LDAP_DEREFERENCE_ALIASES =
+            new EnumGuacamoleProperty<AliasDerefMode>(
+                "never",     AliasDerefMode.NEVER_DEREF_ALIASES,
+                "searching", AliasDerefMode.DEREF_IN_SEARCHING,
+                "finding",   AliasDerefMode.DEREF_FINDING_BASE_OBJ,
+                "always",    AliasDerefMode.DEREF_ALWAYS

Review comment:
       Guessing there's some reason that using `AliasDerefMode.class`, here, doesn't work?

##########
File path: extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/RadiusAuthenticationProtocol.java
##########
@@ -19,100 +19,54 @@
 
 package org.apache.guacamole.auth.radius.conf;
 
+import org.apache.guacamole.properties.EnumGuacamoleProperty.PropertyValue;
+
 /**
  * This enum represents supported RADIUS authentication protocols for
  * the guacamole-auth-radius extension.
  */
 public enum RadiusAuthenticationProtocol {
     
     /**
-     * Password Authentication Protocol (PAP)
+     * Password Authentication Protocol (PAP).
      */
-    PAP("pap"),
+    @PropertyValue("pap")
+    PAP,
     
     /**
-     * Challenge-Handshake Authentication Protocol (CHAP)
+     * Challenge-Handshake Authentication Protocol (CHAP).
      */
-    CHAP("chap"),
+    @PropertyValue("chap")
+    CHAP,
     
     /**
-     * Microsoft implementation of CHAP, Version 1 (MS-CHAPv1)
+     * Microsoft implementation of CHAP, Version 1 (MS-CHAPv1).
      */
-    MSCHAPv1("mschapv1"),
+    @PropertyValue("mschapv1")
+    MSCHAP_V1,
     
     /**
-     * Microsoft implementation of CHAP, Version 2 (MS-CHAPv2)
+     * Microsoft implementation of CHAP, Version 2 (MS-CHAPv2).
      */
-    MSCHAPv2("mschapv2"),
+    @PropertyValue("mschapv2")
+    MSCHAP_V2,
     
     /**
-     * Extensible Authentication Protocol (EAP) with MD5 Hashing (EAP-MD5)
+     * Extensible Authentication Protocol (EAP) with MD5 Hashing (EAP-MD5).
      */
-    EAP_MD5("eap-md5"),
+    @PropertyValue("eap-md5")
+    EAP_MD5,
 
     /**
      * Extensible Authentication Protocol (EAP) with TLS encryption (EAP-TLS).
      */
-    EAP_TLS("eap-tls"),
+    @PropertyValue("eap-tls")
+    EAP_TLS,
 
     /**
      * Extensible Authentication Protocol (EAP) with Tunneled TLS (EAP-TTLS).
      */
-    EAP_TTLS("eap-ttls");
+    @PropertyValue("eap-ttls")
+    EAP_TTLS;
 
-    /**
-     * This variable stores the string value of the protocol, and is also
-     * used within the extension to pass to JRadius for configuring the
-     * library to talk to the RADIUS server.
-     */
-    private final String strValue;
-    
-    /**
-     * Create a new RadiusAuthenticationProtocol object having the
-     * given string value.
-     * 
-     * @param strValue
-     *     The value of the protocol to store as a string, which will be used
-     *     in specifying the protocol within the guacamole.properties file, and
-     *     will also be used by the JRadius library for its configuration.

Review comment:
       I'll have to take a look...

##########
File path: guacamole-ext/src/test/java/org/apache/guacamole/properties/EnumGuacamolePropertyTest.java
##########
@@ -0,0 +1,372 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.properties;
+
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.properties.EnumGuacamoleProperty.PropertyValue;
+import static org.junit.Assert.*;
+import org.junit.Test;
+
+/**
+ * Test which verifies that EnumGuacamoleProperty functions correctly.
+ */
+public class EnumGuacamolePropertyTest {
+
+    /**
+     * Example enum consisting of a small set of possible fish. All values of
+     * this enum are annotated with {@link PropertyValue}.
+     */
+    public static enum Fish {
+
+        /**
+         * Salmon are large, anadromous fish prized for their pink/red/orange
+         * flesh.
+         *
+         * @see <a href="https://en.wikipedia.org/wiki/Salmon">Salmon (Wikipedia)</a>
+         */
+        @PropertyValue("salmon")
+        SALMON,
+
+        /**
+         * Trout are freshwater fish related to salmon, popular both as food
+         * and as game fish.
+         *
+         * @see <a href="https://en.wikipedia.org/wiki/Trout">Trout (Wikipedia)</a>
+         */
+        @PropertyValue("trout")
+        TROUT,
+
+        /**
+         * Mackerel are pelagic fish, typically having vertical stripes along
+         * their backs.
+         *
+         * @see <a href="https://en.wikipedia.org/wiki/Mackerel">Mackerel (Wikipedia)</a>
+         */
+        @PropertyValue("mackerel")
+        MACKEREL,
+
+        /**
+         * Tuna are large, predatory, saltwater fish in the same family as
+         * mackerel. They are one of the few fish that can maintain a body
+         * temperature higher than the surrounding water.
+         *
+         * @see <a href="https://en.wikipedia.org/wiki/Tuna">Tuna (Wikipedia)</a>
+         */
+        @PropertyValue("tuna")
+        TUNA,
+
+        /**
+         * Sardines are small, herring-like fish commonly served in cans.
+         * Sardines are considered prey fish and feed almost exclusively on
+         * zooplankton.
+         *
+         * @see <a href="https://en.wikipedia.org/wiki/Sardine">Sardine (Wikipedia)</a>
+         */
+        @PropertyValue("sardine")
+        SARDINE
+
+    }
+
+    /**
+     * Example enum consisting of a small set of possible vegetables. None of
+     * the values of this enum are annotated with {@link PropertyValue}.
+     */
+    public static enum Vegetable {
+
+        /**
+         * Potatoes are starchy root vegetables native to the Americas. The
+         * tuber itself is edible, but other parts can be toxic.
+         *
+         * @see <a href="https://en.wikipedia.org/wiki/Potato">Potato (Wikipedia)</a>
+         */
+        POTATO,
+
+        /**
+         * Carrots are root vegetables, tapered in shape and generally orange
+         * in color.
+         *
+         * @see <a href="https://en.wikipedia.org/wiki/Carrot">Carrot (Wikipedia)</a>
+         */
+        CARROT
+
+    }
+
+    /**
+     * Example Guacamole property which parses String values as Fish constants.
+     */
+    private static final EnumGuacamoleProperty<Fish> FAVORITE_FISH = new EnumGuacamoleProperty<Fish>(Fish.class) {
+
+        @Override
+        public String getName() {
+            return "favorite-fish";
+        }
+
+    };
+
+    /**
+     * Verifies that EnumGuacamoleProperty correctly parses string values that
+     * are associated with their corresponding enum constants using the
+     * {@link PropertyValue} annotation.
+     *
+     * @throws GuacamoleException
+     *     If a valid test value is incorrectly recognized by parseValue() as
+     *     invalid.
+     */
+    @Test
+    public void testParseValue() throws GuacamoleException {
+        assertEquals(Fish.SALMON,   FAVORITE_FISH.parseValue("salmon"));
+        assertEquals(Fish.TROUT,    FAVORITE_FISH.parseValue("trout"));
+        assertEquals(Fish.MACKEREL, FAVORITE_FISH.parseValue("mackerel"));
+        assertEquals(Fish.TUNA,     FAVORITE_FISH.parseValue("tuna"));
+        assertEquals(Fish.SARDINE,  FAVORITE_FISH.parseValue("sardine"));
+    }
+
+    /**
+     * Verifies that the absence of a property value (null) is parsed by
+     * EnumGuacamoleProperty as the absence of an enum constant (also null).
+     *
+     * @throws GuacamoleException
+     *     If a valid test value is incorrectly recognized by parseValue() as
+     *     invalid.
+     */
+    @Test
+    public void testParseNullValue() throws GuacamoleException {
+        assertNull(FAVORITE_FISH.parseValue(null));
+    }
+
+    /**
+     * Verifies that GuacamoleException is thrown when attempting to parse an
+     * invalid value, and that the error message contains a sorted list of all
+     * allowed values.
+     */
+    @Test
+    public void testParseInvalidValue() {
+        try {
+            FAVORITE_FISH.parseValue("anchovy");
+            fail("Invalid EnumGuacamoleProperty values should fail to parse with an exception.");

Review comment:
       Suggested change:
   `fail("It isn't possible for anchovies to be anyone's favorite fish.");
   
   ;-)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #519: GUACAMOLE-728: Implement EnumGuacamoleProperty.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #519:
URL: https://github.com/apache/guacamole-client/pull/519#discussion_r439776806



##########
File path: extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/RadiusAuthenticationProtocol.java
##########
@@ -19,100 +19,54 @@
 
 package org.apache.guacamole.auth.radius.conf;
 
+import org.apache.guacamole.properties.EnumGuacamoleProperty.PropertyValue;
+
 /**
  * This enum represents supported RADIUS authentication protocols for
  * the guacamole-auth-radius extension.
  */
 public enum RadiusAuthenticationProtocol {
     
     /**
-     * Password Authentication Protocol (PAP)
+     * Password Authentication Protocol (PAP).
      */
-    PAP("pap"),
+    @PropertyValue("pap")
+    PAP,
     
     /**
-     * Challenge-Handshake Authentication Protocol (CHAP)
+     * Challenge-Handshake Authentication Protocol (CHAP).
      */
-    CHAP("chap"),
+    @PropertyValue("chap")
+    CHAP,
     
     /**
-     * Microsoft implementation of CHAP, Version 1 (MS-CHAPv1)
+     * Microsoft implementation of CHAP, Version 1 (MS-CHAPv1).
      */
-    MSCHAPv1("mschapv1"),
+    @PropertyValue("mschapv1")
+    MSCHAP_V1,
     
     /**
-     * Microsoft implementation of CHAP, Version 2 (MS-CHAPv2)
+     * Microsoft implementation of CHAP, Version 2 (MS-CHAPv2).
      */
-    MSCHAPv2("mschapv2"),
+    @PropertyValue("mschapv2")
+    MSCHAP_V2,
     
     /**
-     * Extensible Authentication Protocol (EAP) with MD5 Hashing (EAP-MD5)
+     * Extensible Authentication Protocol (EAP) with MD5 Hashing (EAP-MD5).
      */
-    EAP_MD5("eap-md5"),
+    @PropertyValue("eap-md5")
+    EAP_MD5,
 
     /**
      * Extensible Authentication Protocol (EAP) with TLS encryption (EAP-TLS).
      */
-    EAP_TLS("eap-tls"),
+    @PropertyValue("eap-tls")
+    EAP_TLS,
 
     /**
      * Extensible Authentication Protocol (EAP) with Tunneled TLS (EAP-TTLS).
      */
-    EAP_TTLS("eap-ttls");
+    @PropertyValue("eap-ttls")
+    EAP_TTLS;
 
-    /**
-     * This variable stores the string value of the protocol, and is also
-     * used within the extension to pass to JRadius for configuring the
-     * library to talk to the RADIUS server.
-     */
-    private final String strValue;
-    
-    /**
-     * Create a new RadiusAuthenticationProtocol object having the
-     * given string value.
-     * 
-     * @param strValue
-     *     The value of the protocol to store as a string, which will be used
-     *     in specifying the protocol within the guacamole.properties file, and
-     *     will also be used by the JRadius library for its configuration.

Review comment:
       Ah, but "properly" there means into whatever value is required by JRadius.
   
   I'll restore a means of getting the authenticator name specific to JRadius from the protocol.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #519: GUACAMOLE-728: Implement EnumGuacamoleProperty.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #519:
URL: https://github.com/apache/guacamole-client/pull/519#discussion_r439776587



##########
File path: extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/conf/LDAPGuacamoleProperties.java
##########
@@ -181,8 +183,13 @@ private LDAPGuacamoleProperties() {}
      * Property that controls whether or not the LDAP connection follows
      * (dereferences) aliases as it searches the tree.
      */
-    public static final DereferenceAliasesProperty LDAP_DEREFERENCE_ALIASES =
-            new DereferenceAliasesProperty() {
+    public static final EnumGuacamoleProperty<AliasDerefMode> LDAP_DEREFERENCE_ALIASES =
+            new EnumGuacamoleProperty<AliasDerefMode>(
+                "never",     AliasDerefMode.NEVER_DEREF_ALIASES,
+                "searching", AliasDerefMode.DEREF_IN_SEARCHING,
+                "finding",   AliasDerefMode.DEREF_FINDING_BASE_OBJ,
+                "always",    AliasDerefMode.DEREF_ALWAYS

Review comment:
       Yep, because `AliasDerefMode` is part of an API outside of this extension and Guacamole. It's not within our power to add `@PropertyValue` annotations there.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] necouchman commented on a change in pull request #519: GUACAMOLE-728: Implement EnumGuacamoleProperty.

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #519:
URL: https://github.com/apache/guacamole-client/pull/519#discussion_r439778216



##########
File path: extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/RadiusAuthenticationProtocol.java
##########
@@ -19,100 +19,54 @@
 
 package org.apache.guacamole.auth.radius.conf;
 
+import org.apache.guacamole.properties.EnumGuacamoleProperty.PropertyValue;
+
 /**
  * This enum represents supported RADIUS authentication protocols for
  * the guacamole-auth-radius extension.
  */
 public enum RadiusAuthenticationProtocol {
     
     /**
-     * Password Authentication Protocol (PAP)
+     * Password Authentication Protocol (PAP).
      */
-    PAP("pap"),
+    @PropertyValue("pap")
+    PAP,
     
     /**
-     * Challenge-Handshake Authentication Protocol (CHAP)
+     * Challenge-Handshake Authentication Protocol (CHAP).
      */
-    CHAP("chap"),
+    @PropertyValue("chap")
+    CHAP,
     
     /**
-     * Microsoft implementation of CHAP, Version 1 (MS-CHAPv1)
+     * Microsoft implementation of CHAP, Version 1 (MS-CHAPv1).
      */
-    MSCHAPv1("mschapv1"),
+    @PropertyValue("mschapv1")
+    MSCHAP_V1,
     
     /**
-     * Microsoft implementation of CHAP, Version 2 (MS-CHAPv2)
+     * Microsoft implementation of CHAP, Version 2 (MS-CHAPv2).
      */
-    MSCHAPv2("mschapv2"),
+    @PropertyValue("mschapv2")
+    MSCHAP_V2,
     
     /**
-     * Extensible Authentication Protocol (EAP) with MD5 Hashing (EAP-MD5)
+     * Extensible Authentication Protocol (EAP) with MD5 Hashing (EAP-MD5).
      */
-    EAP_MD5("eap-md5"),
+    @PropertyValue("eap-md5")
+    EAP_MD5,
 
     /**
      * Extensible Authentication Protocol (EAP) with TLS encryption (EAP-TLS).
      */
-    EAP_TLS("eap-tls"),
+    @PropertyValue("eap-tls")
+    EAP_TLS,
 
     /**
      * Extensible Authentication Protocol (EAP) with Tunneled TLS (EAP-TTLS).
      */
-    EAP_TTLS("eap-ttls");
+    @PropertyValue("eap-ttls")
+    EAP_TTLS;
 
-    /**
-     * This variable stores the string value of the protocol, and is also
-     * used within the extension to pass to JRadius for configuring the
-     * library to talk to the RADIUS server.
-     */
-    private final String strValue;
-    
-    /**
-     * Create a new RadiusAuthenticationProtocol object having the
-     * given string value.
-     * 
-     * @param strValue
-     *     The value of the protocol to store as a string, which will be used
-     *     in specifying the protocol within the guacamole.properties file, and
-     *     will also be used by the JRadius library for its configuration.

Review comment:
       Okay - I basically implemented it such that the value set in `guacamole.properties` is the same value that `JRadius` uses, so it just need to be able to retrieve that value and pass it through.  If there's a way to do that mapping back to the `@PropertyValue` annotation I'm not opposed to that.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #519: GUACAMOLE-728: Implement EnumGuacamoleProperty.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #519:
URL: https://github.com/apache/guacamole-client/pull/519#discussion_r439190984



##########
File path: extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/RadiusAuthenticationProtocol.java
##########
@@ -19,100 +19,54 @@
 
 package org.apache.guacamole.auth.radius.conf;
 
+import org.apache.guacamole.properties.EnumGuacamoleProperty.PropertyValue;
+
 /**
  * This enum represents supported RADIUS authentication protocols for
  * the guacamole-auth-radius extension.
  */
 public enum RadiusAuthenticationProtocol {
     
     /**
-     * Password Authentication Protocol (PAP)
+     * Password Authentication Protocol (PAP).
      */
-    PAP("pap"),
+    @PropertyValue("pap")
+    PAP,
     
     /**
-     * Challenge-Handshake Authentication Protocol (CHAP)
+     * Challenge-Handshake Authentication Protocol (CHAP).
      */
-    CHAP("chap"),
+    @PropertyValue("chap")
+    CHAP,
     
     /**
-     * Microsoft implementation of CHAP, Version 1 (MS-CHAPv1)
+     * Microsoft implementation of CHAP, Version 1 (MS-CHAPv1).
      */
-    MSCHAPv1("mschapv1"),
+    @PropertyValue("mschapv1")
+    MSCHAP_V1,
     
     /**
-     * Microsoft implementation of CHAP, Version 2 (MS-CHAPv2)
+     * Microsoft implementation of CHAP, Version 2 (MS-CHAPv2).
      */
-    MSCHAPv2("mschapv2"),
+    @PropertyValue("mschapv2")
+    MSCHAP_V2,
     
     /**
-     * Extensible Authentication Protocol (EAP) with MD5 Hashing (EAP-MD5)
+     * Extensible Authentication Protocol (EAP) with MD5 Hashing (EAP-MD5).
      */
-    EAP_MD5("eap-md5"),
+    @PropertyValue("eap-md5")
+    EAP_MD5,
 
     /**
      * Extensible Authentication Protocol (EAP) with TLS encryption (EAP-TLS).
      */
-    EAP_TLS("eap-tls"),
+    @PropertyValue("eap-tls")
+    EAP_TLS,
 
     /**
      * Extensible Authentication Protocol (EAP) with Tunneled TLS (EAP-TTLS).
      */
-    EAP_TTLS("eap-ttls");
+    @PropertyValue("eap-ttls")
+    EAP_TTLS;
 
-    /**
-     * This variable stores the string value of the protocol, and is also
-     * used within the extension to pass to JRadius for configuring the
-     * library to talk to the RADIUS server.
-     */
-    private final String strValue;
-    
-    /**
-     * Create a new RadiusAuthenticationProtocol object having the
-     * given string value.
-     * 
-     * @param strValue
-     *     The value of the protocol to store as a string, which will be used
-     *     in specifying the protocol within the guacamole.properties file, and
-     *     will also be used by the JRadius library for its configuration.

Review comment:
       Eep - will deleting this break RADIUS? I don't recall the string value being used for JRadius, but if that's the case, I should definitely put some of this back.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [guacamole-client] necouchman merged pull request #519: GUACAMOLE-728: Implement EnumGuacamoleProperty.

Posted by GitBox <gi...@apache.org>.
necouchman merged pull request #519:
URL: https://github.com/apache/guacamole-client/pull/519


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org