mike-jumper commented on code in PR #490:
URL: https://github.com/apache/guacamole-client/pull/490#discussion_r1045300602
##########
guacamole-ext/src/main/java/org/apache/guacamole/properties/GuacamoleProperty.java:
##########
@@ -49,5 +51,25 @@
* provided value.
*/
public Type parseValue(String value) throws GuacamoleException;
+
+ /**
+ * Parses the given string value into a sorted Collection of values of the
+ * type associated with this GuacamoelProperty.
Review Comment:
GuacamoleProperty*
##########
guacamole-ext/src/main/java/org/apache/guacamole/properties/GuacamoleProperty.java:
##########
@@ -49,5 +51,25 @@
* provided value.
*/
public Type parseValue(String value) throws GuacamoleException;
+
+ /**
+ * Parses the given string value into a sorted Collection of values of the
+ * type associated with this GuacamoelProperty.
+ *
+ * @param value
+ * The string value to parse.
+ *
+ * @return
+ * A sorted Collection of the parsed values.
+ *
+ * @throws GuacamoleException
+ * If an error occurs while parsing the provided value.
+ */
+ default public Collection<Type> parseValueCollection(String value)
+ throws GuacamoleException {
+
+ throw new GuacamoleServerException("Collection parsing not implemented
for this property type.");
Review Comment:
This should be a `GuacamoleUnsupportedException`. Another option could be
something like `Collections.singletonList(parseValue(value))`.
##########
guacamole-ext/src/main/java/org/apache/guacamole/environment/Environment.java:
##########
@@ -118,20 +124,103 @@ public <Type> Type getProperty(GuacamoleProperty<Type>
property)
* property in guacamole.properties, if any. If no value is found, the
* provided default value is returned.
*
- * @param <Type> The type that the given property is parsed into.
- * @param property The property to read from guacamole.properties.
- * @param defaultValue The value to return if no value was given in
- * guacamole.properties.
- * @return The parsed value of the property as read from
- * guacamole.properties, or the provided default value if no value
- * was found.
- * @throws GuacamoleException If an error occurs while parsing the value
- * for the given property in
- * guacamole.properties.
+ * @param <Type>
+ * The type that the given property is parsed into.
+ *
+ * @param property
+ * The property to read from guacamole.properties.
+ *
+ * @param defaultValue
+ * The value to return if no value was given in guacamole.properties.
+ *
+ * @return
+ * The parsed value of the property as read from guacamole.properties,
+ * or the provided default value if no value was found.
+ *
+ * @throws GuacamoleException
+ * If an error occurs while parsing the value for the given property in
+ * guacamole.properties.
*/
public <Type> Type getProperty(GuacamoleProperty<Type> property,
Type defaultValue) throws GuacamoleException;
+ /**
+ * Given a GuacamoleProperty, parses and returns a sorted Collection of the
+ * value set for that property in guacamole.properties, if any.
Review Comment:
As it's up to the property implementation to support being read as a
`Collection`, I think we should document here and in other
`getPropertyCollection()` flavors that not all properties support being read as
multiple values and that it's up to the property implementation to define how
multiple values are parsed, if at all.
##########
guacamole-ext/src/main/java/org/apache/guacamole/properties/GuacamoleProperty.java:
##########
@@ -49,5 +51,25 @@
* provided value.
*/
public Type parseValue(String value) throws GuacamoleException;
+
+ /**
+ * Parses the given string value into a sorted Collection of values of the
+ * type associated with this GuacamoelProperty.
+ *
+ * @param value
+ * The string value to parse.
+ *
+ * @return
+ * A sorted Collection of the parsed values.
+ *
+ * @throws GuacamoleException
+ * If an error occurs while parsing the provided value.
+ */
+ default public Collection<Type> parseValueCollection(String value)
Review Comment:
We should note the default behavior in the documentation such that
implementations that inherit this implementation will have that behavior
documented. It may otherwise be surprising to a developer invoking
`parseValueCollection()` that the property they're using doesn't support being
parsed as a collection.
##########
guacamole-ext/src/main/java/org/apache/guacamole/properties/GuacamoleProperty.java:
##########
@@ -49,5 +51,25 @@
* provided value.
*/
public Type parseValue(String value) throws GuacamoleException;
+
+ /**
+ * Parses the given string value into a sorted Collection of values of the
Review Comment:
What do you mean by "sorted" here? Is the order of the values not determined
by the property implementation?
##########
guacamole-ext/src/main/java/org/apache/guacamole/properties/URIListGuacamoleProperty.java:
##########
@@ -0,0 +1,78 @@
+/*
+ * 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 java.net.URI;
+import java.net.URISyntaxException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.regex.Pattern;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleServerException;
+
+/**
+ * A GuacamoleProperty whose value is a List of URIs. The string value
+ * parsed to produce this list is a semicolon-delimited list.
+ * Duplicate values are ignored, as is any whitespace following delimiters.
+ * To maintain compatibility with the behavior of Java properties in general,
+ * only whitespace at the beginning of each value is ignored; trailing
+ * whitespace becomes part of the value.
+ */
+public abstract class URIListGuacamoleProperty
+ implements GuacamoleProperty<List<URI>> {
Review Comment:
Looks good, however in the context of these changes I think this should be
implemented via adding collection support to the existing
`URIGuacamoleProperty`.
##########
guacamole-ext/src/main/java/org/apache/guacamole/environment/Environment.java:
##########
@@ -118,20 +124,103 @@ public <Type> Type getProperty(GuacamoleProperty<Type>
property)
* property in guacamole.properties, if any. If no value is found, the
* provided default value is returned.
*
- * @param <Type> The type that the given property is parsed into.
- * @param property The property to read from guacamole.properties.
- * @param defaultValue The value to return if no value was given in
- * guacamole.properties.
- * @return The parsed value of the property as read from
- * guacamole.properties, or the provided default value if no value
- * was found.
- * @throws GuacamoleException If an error occurs while parsing the value
- * for the given property in
- * guacamole.properties.
+ * @param <Type>
+ * The type that the given property is parsed into.
+ *
+ * @param property
+ * The property to read from guacamole.properties.
+ *
+ * @param defaultValue
+ * The value to return if no value was given in guacamole.properties.
+ *
+ * @return
+ * The parsed value of the property as read from guacamole.properties,
+ * or the provided default value if no value was found.
+ *
+ * @throws GuacamoleException
+ * If an error occurs while parsing the value for the given property in
+ * guacamole.properties.
*/
public <Type> Type getProperty(GuacamoleProperty<Type> property,
Type defaultValue) throws GuacamoleException;
+ /**
+ * Given a GuacamoleProperty, parses and returns a sorted Collection of the
+ * value set for that property in guacamole.properties, if any.
+ *
+ * @param <Type>
+ * The type that the given property is parsed into.
+ *
+ * @param property
+ * The property to read from guacamole.properties.
+ *
+ * @return
+ * A sorted collection of the the parsed values of the property as read
+ * from guacamole.properties.
+ *
+ * @throws GuacamoleException
+ * If an error occurs while parsing the value for the given property in
+ * guacamole.properties.
+ */
+ public <Type> Collection<Type> getPropertyCollection(
+ GuacamoleProperty<Type> property) throws GuacamoleException;
Review Comment:
As this is an `interface`, we'll need to provide `default` implementations
for sake of backward compatibility. I suggest:
1. Internally pulling the raw value as a `String` leveraging
`StringGuacamoleProperty`.
2. Passing that raw value to `parseValueCollection()` of `property` to get
the requested, parsed value.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
