Fix stuck /time fetch, callback threading and body checks in HttpObject

lib/src/main/java/io/ably/pubsub/liveobjects/HttpObject.java

@@ -70,7 +70,8 @@ public interface HttpObject {
      *
      * @param params the request params, built with {@link io.ably.pubsub.liveobjects.http.HttpObjectGetParams},
      *               or {@code null} to fetch the channel object.
-     * @return the compact representation, or {@code null} if the response has no body.
+     * @return the compact representation, or {@code null} if the response has no body; a JSON
+     *         {@code null} value is {@link com.google.gson.JsonNull}.
      * @throws AblyException if the request fails or the response can't be decoded.
      */
     @Blocking

lib/src/main/java/io/ably/pubsub/liveobjects/http/DefaultHttpObject.java

@@ -102,7 +102,8 @@ public void getAsync(@Nullable Param[] params, @NotNull Callback<HttpObjectGetFu
     /** RSO2f - the compact body as JSON, otherwise unchanged; binary values become Base64 strings. */
     private HttpRequester.Request<JsonElement> compactImpl(@Nullable Param[] params) {
         // compact is the server default, so it isn't sent (RSO2d)
-        return getImpl(params, null, body -> body == null ? null : HttpObjectCodec.toJson(body));
+        // a JSON null body is JsonNull, distinct from no body (null)
+        return getImpl(params, null, HttpObjectCodec::toJson);
     }
 
     /** RSO2g - the full body decoded into typed results. */
@@ -113,9 +114,11 @@ private HttpRequester.Request<HttpObjectGetFullResult> fullImpl(@Nullable Param[
 
     private <T> HttpRequester.Request<T> getImpl(@Nullable Param[] params, @Nullable Param format,
                                                  BodyDecoder<T> decoder) {
-        // RSO2c - the objectId, if any, is a percent-encoded path segment
+        // RSO2c - the objectId, if any, is a percent-encoded path segment; an empty one is treated
+        // as absent, as in ably-js, rather than producing a trailing slash
         String objectId = Param.getFirst(params, HttpObjectGetParams.OBJECT_ID);
-        final String path = objectId != null ? basePath + "/" + HttpUtils.encodeURIComponent(objectId) : basePath;
+        final String path = objectId != null && !objectId.isEmpty()
+            ? basePath + "/" + HttpUtils.encodeURIComponent(objectId) : basePath;
 
         // RSO2d, RSO6 - every other param, such as path, goes in the query string unmodified; the
         // format is this method's, so a caller's compact param is dropped
@@ -130,9 +133,12 @@ private <T> HttpRequester.Request<T> getImpl(@Nullable Param[] params, @Nullable
         if (client.options.addRequestIds) query.add(Crypto.generateRandomRequestId()); // RSC7c
         final Param[] queryParams = query.toArray(new Param[0]);
 
-        // RSO2e - the body is decoded per RSC8 before the decoder runs; no body is null
+        // RSO2e - the body is decoded per RSC8 before the decoder runs; no body is a null result
+        final HttpCore.ResponseHandler<T> handler = responseHandler(decoder);
         return client.http.request((http, callback) ->
-            http.get(path, HttpUtils.defaultAcceptHeaders(binary()), queryParams, responseHandler(decoder), true, callback));
+            http.get(path, HttpUtils.defaultAcceptHeaders(binary()), queryParams, (response, error) ->
+                error == null && (response.body == null || response.body.length == 0)
+                    ? null : handler.handleResponse(response, error), true, callback));
     }
 
     //endregion
@@ -192,7 +198,7 @@ private HttpCore.RequestBody encodeOperations(List<HttpObjectOperation> operatio
             return HttpObjectCodec.requestBody(wireOperations, binary());
         } catch (RuntimeException e) {
             // e.g. a null where the typed API expects a value; report it as invalid input
-            throw HttpObjectCodec.invalidInput("Invalid operation: " + e);
+            throw HttpObjectCodec.invalidInput("Invalid operation: " + HttpObjectCodec.describe(e));
         }
     }
 
@@ -219,15 +225,17 @@ public void generateObjectIdAsync(@NotNull final HttpObjectGenerateIdBody create
                 cb.onError(e.errorInfo);
                 return;
             }
+            // A waiter can be notified on the thread of a blocking generateObjectId that fetched the
+            // time, so both outcomes are handed back to the scheduler.
             serverTime.currentTimeMillisAsync(new Callback<Long>() {
                 @Override
                 public void onSuccess(Long timestamp) {
-                    cb.onSuccess(body.withTimestamp(timestamp));
+                    http.execute(() -> cb.onSuccess(body.withTimestamp(timestamp)));
                 }
 
                 @Override
                 public void onError(ErrorInfo reason) {
-                    cb.onError(reason);
+                    http.execute(() -> cb.onError(reason));
                 }
             });
         })).async(callback);
@@ -251,7 +259,7 @@ static CreateBody of(HttpObjectGenerateIdBody createBody) throws AblyException {
                 return encode(createBody);
             } catch (RuntimeException e) {
                 // e.g. a null entry value; report it as invalid input
-                throw HttpObjectCodec.invalidInput("Invalid create body: " + e);
+                throw HttpObjectCodec.invalidInput("Invalid create body: " + HttpObjectCodec.describe(e));
             }
         }
 

lib/src/main/java/io/ably/pubsub/liveobjects/http/HttpLiveObject.java

@@ -22,7 +22,7 @@ public interface HttpLiveObject extends HttpObjectGetFullResult {
      * For an unrecognised object type this holds its type-specific data. Values are JSON, as in a
      * {@code getCompact} result: binary values are Base64 strings for both protocols.
      *
-     * @return an unmodifiable map of field name to value; empty if there are none.
+     * @return a copy of the fields, by name; changes to it are not reflected. Empty if there are none.
      */
     @NotNull JsonObject getUnrecognizedFields();
 }

lib/src/main/java/io/ably/pubsub/liveobjects/http/HttpObjectCodec.java

@@ -12,6 +12,7 @@
 import io.ably.pubsub.types.AblyException;
 import io.ably.pubsub.types.ErrorInfo;
 import io.ably.pubsub.util.Base64Coder;
+import io.ably.pubsub.util.Log;
 import org.msgpack.core.MessageBufferPacker;
 import org.msgpack.core.MessagePack;
 import org.msgpack.core.MessagePacker;
@@ -40,6 +41,7 @@
  */
 final class HttpObjectCodec {
 
+    private static final String TAG = HttpObjectCodec.class.getName();
     private static final String MSGPACK_CONTENT_TYPE = "application/x-msgpack";
     private static final Charset UTF_8 = Charset.forName("UTF-8");
     private static final List<String> LEAF_VALUE_FIELDS = Arrays.asList("string", "number", "boolean", "bytes", "json");
@@ -54,6 +56,12 @@ private HttpObjectCodec() {}
      * field (RSO3d4).
      */
     static Map<String, Object> encodeOperation(HttpObjectOperation op, boolean binary) throws AblyException {
+        // RSO5b1 - the factories set exactly one body, but a null passed to one leaves none
+        int bodies = count(op.getMapCreate(), op.getMapCreateWithObjectId(), op.getMapSet(), op.getMapRemove(),
+            op.getCounterCreate(), op.getCounterCreateWithObjectId(), op.getCounterInc());
+        if (bodies != 1) {
+            throw invalidInput("An operation must have exactly one operation body, but it has " + bodies);
+        }
         Map<String, Object> wire = new LinkedHashMap<>();
         putIfNotNull(wire, "id", op.getId());
         putIfNotNull(wire, "objectId", op.getObjectId());
@@ -66,12 +74,12 @@ static Map<String, Object> encodeOperation(HttpObjectOperation op, boolean binar
         }
         if (op.getMapSet() != null) {
             Map<String, Object> mapSet = new LinkedHashMap<>();
-            mapSet.put("key", op.getMapSet().getKey());
+            mapSet.put("key", notNull(op.getMapSet().getKey(), "mapSet.key"));
             mapSet.put("value", encodeData(op.getMapSet().getValue(), binary));
             wire.put("mapSet", mapSet);
         }
         if (op.getMapRemove() != null) {
-            wire.put("mapRemove", singleton("key", op.getMapRemove().getKey()));
+            wire.put("mapRemove", singleton("key", notNull(op.getMapRemove().getKey(), "mapRemove.key")));
         }
         if (op.getCounterCreate() != null) {
             wire.put("counterCreate", encodeCounterCreate(op.getCounterCreate()));
@@ -89,7 +97,7 @@ static Map<String, Object> encodeOperation(HttpObjectOperation op, boolean binar
     static Map<String, Object> encodeMapCreate(HttpObjectOperation.MapCreate body, boolean binary) throws AblyException {
         Map<String, Object> entries = new LinkedHashMap<>();
         for (Map.Entry<String, PublishObjectData> entry : body.getEntries().entrySet()) {
-            entries.put(entry.getKey(), singleton("data", encodeData(entry.getValue(), binary)));
+            entries.put(notNull(entry.getKey(), "mapCreate.entries key"), singleton("data", encodeData(entry.getValue(), binary)));
         }
         Map<String, Object> mapCreate = new LinkedHashMap<>();
         mapCreate.put("semantics", encodeSemantics(body.getSemantics()));
@@ -101,7 +109,10 @@ static Map<String, Object> encodeCounterCreate(HttpObjectOperation.CounterCreate
         return singleton("count", finite(body.getCount(), "counterCreate.count"));
     }
 
-    private static Map<String, Object> encodeCreateWithObjectId(HttpObjectOperation.CreateWithObjectId body) {
+    private static Map<String, Object> encodeCreateWithObjectId(HttpObjectOperation.CreateWithObjectId body) throws AblyException {
+        if (body.getInitialValue() == null || body.getNonce() == null) {
+            throw invalidInput("initialValue and nonce must not be null; use the values from generateObjectId");
+        }
         Map<String, Object> wire = new LinkedHashMap<>();
         wire.put("initialValue", body.getInitialValue());
         wire.put("nonce", body.getNonce());
@@ -208,7 +219,8 @@ private static HttpLiveCounter decodeCounter(String objectId, Map<?, ?> counter,
      * OMP2 code ({@code 0}); accept both. Anything else is preserved as UNKNOWN.
      */
     private static ObjectsMapSemantics decodeSemantics(Object value) {
-        if (value instanceof Number && ((Number) value).longValue() == 0L) return ObjectsMapSemantics.LWW;
+        // integral zero only: longValue() would also truncate 0.5 or a BigInteger of 2^64 to 0
+        if (value instanceof Long && (Long) value == 0L) return ObjectsMapSemantics.LWW;
         if (value instanceof String && "lww".equalsIgnoreCase((String) value)) return ObjectsMapSemantics.LWW;
         return ObjectsMapSemantics.UNKNOWN;
     }
@@ -217,13 +229,22 @@ private static ObjectsMapSemantics decodeSemantics(Object value) {
      * Decodes a leaf per OD5 (RSO2g3): Base64 {@code bytes} become binary (OD5b2), and the
      * JSON-encoded string in {@code json} is parsed (OD5a2, OD5b3). The result carries no
      * {@code objectId} or {@code encoding} (RSO9c1).
+     *
+     * <p>A {@code json} or {@code bytes} value that can't be decoded is logged and left out, so the
+     * leaf decodes as {@link io.ably.pubsub.liveobjects.ValueType#UNKNOWN} rather than failing the
+     * whole response. Like ably-js, which returns that leaf's raw fields, only the bad leaf is
+     * affected; the raw value isn't exposed, as {@link HttpObjectData} has no accessor for it.
      */
     private static HttpObjectData decodeLeaf(Map<?, ?> fields) {
         final String string = asString(fields.get("string"));
         JsonElement json = null;
         Object rawJson = fields.get("json");
         if (rawJson instanceof String) {
-            json = JsonParser.parseString((String) rawJson);
+            try {
+                json = JsonParser.parseString((String) rawJson);
+            } catch (RuntimeException e) {
+                Log.e(TAG, "Leaf json value is not valid JSON; leaving it out", e);
+            }
         } else if (rawJson != null) {
             json = toJson(rawJson);
         }
@@ -232,7 +253,11 @@ private static HttpObjectData decodeLeaf(Map<?, ?> fields) {
         if (rawBytes instanceof byte[]) {
             bytes = (byte[]) rawBytes;
         } else if (rawBytes instanceof String) {
-            bytes = Base64Coder.decode((String) rawBytes);
+            try {
+                bytes = Base64Coder.decode((String) rawBytes);
+            } catch (RuntimeException e) {
+                Log.e(TAG, "Leaf bytes value is not valid Base64; leaving it out", e);
+            }
         }
         Object number = fields.get("number");
         Object bool = fields.get("boolean");
@@ -373,7 +398,10 @@ private static void pack(MessagePacker packer, Object value) throws IOException
             packer.packString((String) value);
         } else if (value instanceof Boolean) {
             packer.packBoolean((Boolean) value);
-        } else if (value instanceof Double || value instanceof Float || value instanceof BigDecimal) {
+        } else if (value instanceof Float) {
+            // via the decimal string, as JSON writes it: widening 0.1f directly gives 0.10000000149011612
+            packer.packDouble(Double.parseDouble(value.toString()));
+        } else if (value instanceof Double || value instanceof BigDecimal) {
             packer.packDouble(((Number) value).doubleValue());
         } else if (value instanceof BigInteger) {
             packer.packBigInteger((BigInteger) value);
@@ -464,6 +492,24 @@ private static Number finite(Number number, String field) throws AblyException {
         return number;
     }
 
+    private static <V> V notNull(V value, String field) throws AblyException {
+        if (value == null) throw invalidInput("Invalid " + field + ": must not be null");
+        return value;
+    }
+
+    /** A short description of an unexpected exception for an invalid-input message. */
+    static String describe(RuntimeException e) {
+        return e.getMessage() != null ? e.getMessage() : e.getClass().getSimpleName();
+    }
+
+    private static int count(Object... values) {
+        int count = 0;
+        for (Object value : values) {
+            if (value != null) count++;
+        }
+        return count;
+    }
+
     private static String asString(Object value) {
         return value instanceof String ? (String) value : null;
     }

lib/src/main/java/io/ably/pubsub/liveobjects/http/HttpObjectGenerateIdBody.java

@@ -49,10 +49,12 @@ private HttpObjectGenerateIdBody(@Nullable HttpObjectOperation.MapCreate mapCrea
         return counterCreate(new HttpObjectOperation.CounterCreate(count));
     }
 
+    /** @return the map create body, or {@code null} if this is a counter create body. */
     public @Nullable HttpObjectOperation.MapCreate getMapCreate() {
         return mapCreate;
     }
 
+    /** @return the counter create body, or {@code null} if this is a map create body. */
     public @Nullable HttpObjectOperation.CounterCreate getCounterCreate() {
         return counterCreate;
     }

lib/src/main/java/io/ably/pubsub/liveobjects/http/HttpObjectGetParams.java

@@ -23,7 +23,7 @@ public final class HttpObjectGetParams {
 
     /**
      * The key of the ID of the object to fetch. It's sent as a path segment, not in the query
-     * string. If it's absent, the channel object is the target.
+     * string. If it's absent or empty, the channel object is the target.
      * <p>Spec: RSO2a1a
      */
     public static final String OBJECT_ID = "objectId";