Clean up base Command and CommandResponse APIs (#41)

* Remove `Command.copy` and unnecessary copy constructors
* Make Response constructors more consistent
* Make `ResponseObject` properties immutable
* Remove long unused `org.labkey.remoteapi.security.ACL`
* Chip away at missing JavaDoc

Author: labkey-tchad

labkey-client-api/CHANGELOG.md

@@ -1,12 +1,16 @@
 # The LabKey Remote API Library for Java - Change Log
 
 ## version 4.0.0
-*Released*: TBD
-* Replace the internal JSON library with [JSON-java](https://github.com/stleary/JSON-java). The previous library,
+*Released*: 26 October 2022
+* Migrate to a new JSON library: [JSON-java](https://github.com/stleary/JSON-java). The previous library,
   [json-simple](https://github.com/fangyidong/json-simple) is no longer maintained (last released in early 2012) and
   lacks support for basic features like generics. This is an incompatible API change for developers who *write* their own
   Command classes; they will need to update their Command classes if/when they upgrade to v4.0.0. Developers who simply
   *use* Command classes should be able to upgrade without changes.
+* API changes:
+  * Remove `copy` method from Commands. _It was inconsistently implemented and served little purpose._
+  * Remove `CommandResponse.getSourceCommand`. _Functionality would vary wildly because of inconsistent `Command.copy` implementations._
+  * Make the properties stashed by `ResponseObject` immutable.
 * Issue 46321: Remove `lib` directory from `fatJar` in favor of pulling dependencies via the published pom files when needed
 * Remove artifactory plugin since we use the maven `publish` command now
 

labkey-client-api/build.gradle

@@ -48,7 +48,7 @@ repositories {
 
 group "org.labkey.api"
 
-version "4.0.0-SNAPSHOT"
+version "4.1.0-SNAPSHOT"
 
 dependencies {
     api "org.json:json:${jsonObjectVersion}"

labkey-client-api/gradle.properties

@@ -17,7 +17,6 @@ hamcrestVersion=1.3
 httpclient5Version=5.1.3
 httpcore5Version=5.1.4
 
-jsonSimpleVersion=1.1.1
 jsonObjectVersion=20220924
 
 junitVersion=4.13.2

labkey-client-api/src/org/labkey/remoteapi/ApiKeyCredentialsProvider.java

@@ -15,15 +15,11 @@
  */
 package org.labkey.remoteapi;
 
-import org.apache.hc.client5.http.auth.AuthenticationException;
 import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
 
 import java.net.URI;
 
-/**
- * Created by adam on 4/15/2016.
- */
 public class ApiKeyCredentialsProvider implements CredentialsProvider
 {
     private final String _apiKey;
@@ -34,7 +30,7 @@ public ApiKeyCredentialsProvider(String apiKey)
     }
 
     @Override
-    public void configureRequest(URI baseURI, HttpUriRequest request, HttpClientContext httpClientContext) throws AuthenticationException
+    public void configureRequest(URI baseURI, HttpUriRequest request, HttpClientContext httpClientContext)
     {
         request.setHeader("apikey", _apiKey);
     }

labkey-client-api/src/org/labkey/remoteapi/ApiVersionException.java

@@ -17,11 +17,6 @@
 
 import org.json.JSONObject;
 
-/**
- * User: adam
- * Date: Feb 23, 2009
- * Time: 1:13:26 PM
- */
 public class ApiVersionException extends CommandException
 {
     ApiVersionException(String message, int statusCode, JSONObject jsonProperties, String responseText, String contentType)

labkey-client-api/src/org/labkey/remoteapi/BasicAuthCredentialsProvider.java

@@ -16,19 +16,14 @@
 package org.labkey.remoteapi;
 
 import org.apache.hc.client5.http.auth.AuthScope;
-import org.apache.hc.client5.http.auth.AuthenticationException;
 import org.apache.hc.client5.http.auth.Credentials;
 import org.apache.hc.client5.http.auth.UsernamePasswordCredentials;
 import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
 import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider;
-import org.apache.hc.client5.http.impl.auth.BasicScheme;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
 
 import java.net.URI;
 
-/**
- * Created by adam on 4/15/2016.
- */
 public class BasicAuthCredentialsProvider implements CredentialsProvider
 {
     private final String _email;
@@ -41,7 +36,7 @@ public BasicAuthCredentialsProvider(String email, String password)
     }
 
     @Override
-    public void configureRequest(URI baseURI, HttpUriRequest request, HttpClientContext httpClientContext) throws AuthenticationException
+    public void configureRequest(URI baseURI, HttpUriRequest request, HttpClientContext httpClientContext)
     {
         BasicCredentialsProvider provider = new BasicCredentialsProvider();
         AuthScope scope = new AuthScope(baseURI.getHost(), -1);

labkey-client-api/src/org/labkey/remoteapi/Command.java

@@ -25,7 +25,14 @@
 import org.json.JSONObject;
 import org.json.JSONTokener;
 
-import java.io.*;
+import java.io.BufferedReader;
+import java.io.ByteArrayInputStream;
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.io.StringReader;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.nio.charset.StandardCharsets;
@@ -45,16 +52,13 @@
  * However, if future versions of the LabKey Server expose new HTTP APIs
  * that are not yet supported with a specialized class in this library,
  * the developer may still invoke these APIs by creating an instance of the
- * Command object directly, providing the controller and action name for
- * the new API. Parameters may then be specified by calling the <code>setParameters()</code>
+ * {@link Command} class directly, providing the controller and action name for
+ * the new API. Parameters may then be specified by calling the {@link #setParameters(Map)}
  * method, passing a populated parameter <code>Map&lt;String, Object&gt;</code>
  * <p>
  * Note that this class is not thread-safe. Do not share instances of this class
  * or its descendants between threads, unless the descendant declares explicitly that
  * it is thread-safe.
- *
- * @author Dave Stearns, LabKey Corporation
- * @version 1.0
  */
 public class Command<ResponseType extends CommandResponse>
 {
@@ -386,7 +390,7 @@ private void throwError(Response r, boolean throwByDefault) throws IOException,
                 json = new JSONObject(responseText);
                 if (json.has("exception"))
                 {
-                    message = (String)json.get("exception");
+                    message = json.getString("exception");
 
                     if ("org.labkey.api.action.ApiVersionException".equals(json.opt("exceptionClass")))
                         throw new ApiVersionException(message, r.getStatusCode(), json, responseText, contentType);
@@ -419,7 +423,7 @@ private void throwError(Response r, boolean throwByDefault) throws IOException,
      */
     protected ResponseType createResponse(String text, int status, String contentType, JSONObject json)
     {
-        return (ResponseType)new CommandResponse(text, status, contentType, json, this.copy());
+        return (ResponseType)new CommandResponse(text, status, contentType, json, this);
     }
 
     /**
@@ -570,14 +574,4 @@ public void setRequiredVersion(double requiredVersion)
         _requiredVersion = requiredVersion;
     }
 
-    /**
-     * Returns a copy of this object. Derived classes should override this
-     * to copy their own data members   
-     * @return A copy of this object
-     */
-    // TODO: For next major release, genericize this return value (Command<ResponseType>)
-    public Command copy()
-    {
-        return new Command<>(this);
-    }
 }

labkey-client-api/src/org/labkey/remoteapi/CommandResponse.java

@@ -22,12 +22,6 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-/*
-* User: Dave
-* Date: Jul 10, 2008
-* Time: 1:21:24 PM
-*/
-
 /**
  * Represents the details of a response returned from the LabKey Server.
  * <p>
@@ -36,16 +30,13 @@
  * SelectRowsResponse). However, if you are using the Command class directly to
  * call an HTTP API that does not yet have a specialized command class,
  * you would use this object to obtain the response details.
- *
- * @author Dave Stearns, LabKey Corporation
- * @version 1.0
  */
 public class CommandResponse
 {
     private final String _text;
     private final int _statusCode;
     private final String _contentType;
-    private final Command _sourceCommand;
+    private final double _requiredVersion;
 
     private Map<String, Object> _data;
 
@@ -58,13 +49,13 @@ public class CommandResponse
      * @param json The parsed JSONObject (or null if JSON was not returned).
      * @param sourceCommand A copy of the command that created this response
      */
-    public CommandResponse(String text, int statusCode, String contentType, JSONObject json, Command sourceCommand)
+    public CommandResponse(String text, int statusCode, String contentType, JSONObject json, Command<?> sourceCommand)
     {
         _text = text;
         _statusCode = statusCode;
         _contentType = contentType;
         _data = null != json ? json.toMap() : null;
-        _sourceCommand = sourceCommand;
+        _requiredVersion = sourceCommand != null ? sourceCommand.getRequiredVersion() : 0.0;
     }
 
     /**
@@ -110,16 +101,7 @@ public String getContentType()
      */
     public double getRequiredVersion()
     {
-        return _sourceCommand.getRequiredVersion();
-    }
-
-    /**
-     * Returns a reference to a copy of the command that created this response
-     * @return The command that created this response
-     */
-    public Command getSourceCommand()
-    {
-        return _sourceCommand;
+        return _requiredVersion;
     }
 
     /**
@@ -131,7 +113,6 @@ public Command getSourceCommand()
      * Lists, or Maps.
      * @return The parsed data as a property map.
      */
-    @SuppressWarnings("unchecked")
     public Map<String, Object> getParsedData()
     {
         if (null == _data && null != getText()
@@ -153,7 +134,6 @@ public Map<String, Object> getParsedData()
      * @param <T> the type of the property.
      * @return The property value, or null if the property was not found.
      */
-    @SuppressWarnings("unchecked")
     public <T> T getProperty(String path)
     {
         assert null != path;
@@ -192,8 +172,8 @@ protected <T> T getProperty(String[] path, int pathIndex, Map<String, Object> pa
         Object prop = parent.get(key);
         if (arrayIndex != null)
         {
-            if (prop instanceof List && ((List) prop).size() > arrayIndex)
-                prop = ((List)prop).get(arrayIndex);
+            if (prop instanceof List<?> list && list.size() > arrayIndex)
+                prop = list.get(arrayIndex);
             else
                 return null;
         }

labkey-client-api/src/org/labkey/remoteapi/Connection.java

@@ -94,9 +94,6 @@
  * Note that this class is not thread-safe. Do not share instances of Connection
  * between threads.
  * </p>
- * 
- * @author Dave Stearns, LabKey Corporation
- * @version 1.0
  */
 public class Connection
 {
@@ -318,35 +315,29 @@ protected void afterExecute()
 
     /**
      * Ensures that the credentials have been used to authenticate the users and returns a client that can be used for other requests
-     * @return an HTTP client
-     * @throws IOException if there is an IO problem executing the command to ensure login
+     *
+     * @throws IOException      if there is an IO problem executing the command to ensure login
      * @throws CommandException if the server returned a non-success status code.
      */
-    // TODO: For next major release, stop returning CloseableHttpClient?
-    public CloseableHttpClient ensureAuthenticated() throws IOException, CommandException
+    public void ensureAuthenticated() throws IOException, CommandException
     {
-        EnsureLoginCommand command = new EnsureLoginCommand();
-        CommandResponse response = command.execute(this, "/home");
-        return getHttpClient();
+        new EnsureLoginCommand().execute(this, "/home");
     }
 
     /**
      * Invalidates the current HTTP session on the server, if any
-     * @return an HTTP client
+     *
      * @throws IOException if there is an IO problem executing the command to ensure logout
      * @throws CommandException if the server returned a non-success status code.
      */
-    // TODO: For next major release, stop returning CloseableHttpClient?
-    public CloseableHttpClient logout() throws IOException, CommandException
+    public void logout() throws IOException, CommandException
     {
-        LogoutCommand command = new LogoutCommand();
-        CommandResponse response = command.execute(this, "/home");
-        return getHttpClient();
+        new LogoutCommand().execute(this, "/home");
     }
 
     /**
      * For site-admins only, start impersonating a user.
-     *
+     * <p>
      * Admins may impersonate other users to perform actions on their behalf.
      * Site admins may impersonate any user in any project. Project admins must
      * provide a <code>projectPath</code> and may only impersonate within the
@@ -365,7 +356,7 @@ public Connection impersonate(/*@NotNull*/ String email) throws IOException, Com
 
     /**
      * For site-admins or project-admins only, start impersonating a user.
-     *
+     * <p>
      * Admins may impersonate other users to perform actions on their behalf.
      * Site admins may impersonate any user in any project. Project admins must
      * provide a <code>projectPath</code> and may only impersonate within the
@@ -420,7 +411,7 @@ public Connection stopImpersonate() throws IOException, CommandException
         return stopImpersonating();
     }
 
-    CloseableHttpResponse executeRequest(HttpUriRequest request, Integer timeout) throws IOException, URISyntaxException, AuthenticationException
+    CloseableHttpResponse executeRequest(HttpUriRequest request, Integer timeout) throws IOException, AuthenticationException
     {
         // Delegate authentication setup to CredentialsProvider
         _credentialsProvider.configureRequest(getBaseURI(), request, _httpClientContext);
@@ -468,8 +459,8 @@ public int getTimeout()
     }
 
     /**
-     * Sets the proxy host and port for this Connection.
-     * NOTE: Changing this setting will force the underlying http client to be recreated.
+     * Sets the proxy host and port for this Connection.<br>
+     * <i>NOTE: Changing this setting will force the underlying http client to be recreated.</i>
      * @param host the proxy host
      * @param port the proxy port
      * @return this connection
@@ -496,8 +487,8 @@ public boolean isAcceptSelfSignedCerts()
     /**
      * Sets the accept-self-signed certificates option. Set to false
      * to disable automatic acceptance of self-signed SSL certificates
-     * when using HTTPS.
-     * NOTE: Changing this setting will force the underlying http client to be recreated.
+     * when using HTTPS.<br>
+     * <i>NOTE: Changing this setting will force the underlying http client to be recreated.</i>
      *
      * @param acceptSelfSignedCerts set to false to not accept self-signed certificates
      * @return this connection

labkey-client-api/src/org/labkey/remoteapi/CredentialsProvider.java

@@ -21,9 +21,6 @@
 
 import java.net.URI;
 
-/**
- * Created by adam on 4/15/2016.
- */
 public interface CredentialsProvider
 {
     void configureRequest(URI baseURI, HttpUriRequest request, HttpClientContext httpClientContext) throws AuthenticationException;