-
Notifications
You must be signed in to change notification settings - Fork 712
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GUACAMOLE-1006: Provide implementations for List properties. #490
Conversation
bafca03
to
3e74bd0
Compare
What do you think about enhancing It could then be up to the |
Sounds good - I will take a look. |
@mike-jumper Okay, I've started down path that I think is approximately what you're suggesting, while trying to avoid breaking everything in the process. So, instead of making This would rely on each of the properties to implement a new Let me know if this looks good, sane, reasonable, etc., or if there's a better solution? I could break everything and make |
guacamole-ext/src/main/java/org/apache/guacamole/environment/Environment.java
Outdated
Show resolved
Hide resolved
guacamole-ext/src/main/java/org/apache/guacamole/properties/URIListGuacamoleProperty.java
Outdated
Show resolved
Hide resolved
guacamole-ext/src/main/java/org/apache/guacamole/properties/GuacamoleProperty.java
Outdated
Show resolved
Hide resolved
guacamole-ext/src/main/java/org/apache/guacamole/properties/GuacamoleProperty.java
Outdated
Show resolved
Hide resolved
guacamole-ext/src/main/java/org/apache/guacamole/properties/GuacamoleProperty.java
Show resolved
Hide resolved
guacamole-ext/src/main/java/org/apache/guacamole/properties/GuacamoleProperty.java
Outdated
Show resolved
Hide resolved
guacamole-ext/src/main/java/org/apache/guacamole/environment/Environment.java
Outdated
Show resolved
Hide resolved
e264889
to
98ef4ae
Compare
@mike-jumper Okay, I've re-worked the implementation, here, providing support within the existing property classes for parsing and retrieving lists of values. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default
implementations for new functions were added to the GuacamoleProperty
interface, but I think they're missing from Environment
.
Otherwise, LGTM!
Okay, I think I managed to get these default methods implemented correctly in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slick approach - I see what you mean about the spoon.
Provides some basic implementations for
String
andURI
list properties inguacamole-ext
, in anticipation of those being required for things like LDAP and RADIUS server redundancy.Also, note that I switched over to semicolon delimiting for these - that's worth some discussion, but my rationale for that is that some URIs - LDAP ones, for instance - rely on the ability to use commas to separate LDAP DNs within the URI, so, one way or the other we need to address that.