Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/guacamole-client/pull/353#discussion_r245747651
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/connection/ConnectionService.java
---
@@ -148,12 +169,25 @@
// Set protocol
GuacamoleConfiguration config = new
GuacamoleConfiguration();
+
+ GuacamoleProxyConfiguration proxyConfig;
+ try {
+ proxyConfig = new
LocalEnvironment().getDefaultGuacamoleProxyConfiguration();
--- End diff --
The `LocalEnvironment` should not be repeatedly recreated if it can be
avoided.
A singleton instance of it is actually already bound during startup of the
LDAP extension, so that would be the instance of choice:
https://github.com/apache/guacamole-client/blob/78f1ae1b4eac25501d532ddee94fd1d8588e56dc/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPAuthenticationProviderModule.java#L74
That singleton instance can be injected within this class if needed, for
example:
https://github.com/apache/guacamole-client/blob/78f1ae1b4eac25501d532ddee94fd1d8588e56dc/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java#L41-L45
Since this is configuration, though, I'd think it would make more sense to
provide this via a function within `ConfigurationService`.
---