Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/131#discussion_r107032658
--- Diff:
extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ConfigurationService.java
---
@@ -223,4 +231,51 @@ public int getMaxResults() throws GuacamoleException {
);
}
+ /**
+ * Returns whether or not LDAP aliases will be dereferenced,
+ * as configured with guacamole.properties. The default
+ * behavior if not explicityly defined is to never
+ * dereference them.
+ *
+ * @return
+ * The behavior for handling dereferencing of aliases
+ * as configured in guacamole.properties.
+ *
+ * @throws GuacamoleException
+ * If guacamole.properties cannot be parsed.
+ */
+ public DereferenceAliases getDereferenceAliases() throws
GuacamoleException {
+ return environment.getProperty(
+ LDAPGuacamoleProperties.LDAP_DEREFERENCE_ALIASES,
+ DereferenceAliases.NEVER
+ );
+
+ }
+
+ /**
+ * Returns a set of LDAPSearchConstraints to apply globally
+ * to all LDAP searches rather than having various instances
+ * dispersed throughout the code. Currently contains the
--- End diff --
I would recommend, simply:
"Returns a set of LDAPSearchConstraints which should be applied to all LDAP
searches."
That a function should be used "rather than [copying the internals of the
function everywhere]" applies to all functions by definition, and goes without
saying. There's no need to enumerate the bad practices we avoid by having
functions.
That said ... if this function effectively replaces the
`getDereferenceAliases()` and `getMaxResults()` functions, and it no longer
makes sense to call those functions externally, perhaps those functions should
be removed (or declared `private`)?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
