[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #546: GUACAMOLE-1123: Standardize REST endpoints for historical data

[GitBox <gi...@apache.org>]

mike-jumper commented on a change in pull request #546:
URL: https://github.com/apache/guacamole-client/pull/546#discussion_r483275361



##########
File path: guacamole-ext/src/main/java/org/apache/guacamole/net/auth/User.java
##########
@@ -103,7 +103,26 @@
      *     If an error occurs while reading the history of this user, or if
      *     permission is denied.
      */
+    @Deprecated

Review comment:
       The context of this deprecation needs to be documented within the JavaDoc as well.

##########
File path: guacamole-ext/src/main/java/org/apache/guacamole/net/auth/User.java
##########
@@ -103,7 +103,26 @@
      *     If an error occurs while reading the history of this user, or if
      *     permission is denied.
      */
+    @Deprecated

Review comment:
       We'll have to be careful here:
   
   * As written, deprecated or not, an implementation of `User` will still be required to implement the older `getHistory()` function.
   * Providing a default implementation of `getHistory()` which leverages `getUserHistory()` would allow things to behave as expected, but then older extensions which implement only `getHistory()` will fail.
   * Providing default implementations of _both_ which each point at the other would would technically work, but would make implementing `User` dangerous. An extension that fails to implement either function would compile without any warning or error, yet invoking either function would result in infinite recursion.
   
   The way we worked around this for similar changes to `Connectable` was to provide a default implementation only for the deprecated function at the library level, but override that internally with an identical interface specific to the web application that provides default implementations of both.
   
   Finding some way to achieve this purely through enhancements to `ActivityRecordSet` could avoid the above.




----------------------------------------------------------------
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