Skip to content

Three REST publish-body assertions contradict features.md (RSL4c3/RSL4d3, RSL1k1, RSL1k3), and eight unit specs read the request body without pinning the protocol #527

Description

@owenpearson

Summary

Three assertions about what a REST publish puts on the wire cannot be satisfied by a conforming SDK:

  • channel/publish.md:129 asserts an object payload travels unstringified, against RSL4c3 and RSL4d3.
  • channel/publish.md:177 asserts whole-body equality against a literal that omits the id RSL1k1 adds while idempotentRestPublishing is on by default.
  • channel/idempotency.md:409 asserts a library-generated id for the id-less message in a mixed batch, which RSL1k1 does not generate and RSL1k3 forbids.

Separately, and not a features.md contradiction: eight rest/unit specs read the request body with parse_json(request.body) without ever pinning useBinaryProtocol, which TO3f defaults to true. That is a gap in the documented conventions rather than a defect in any one test, and the last section says what the ask is.

ably-js and ably-python have each derived all of these. On the first three they diverge from the spec in different directions; on the fourth they diverge from each other, which is the clearest evidence that the convention is not currently written down anywhere a unit-spec author would find it.

What the spec says today

features.md:331 and :336 (RSL4c3, RSL4d3):

  • (RSL4c3) a JSON Message payload is stringified either as a JSON Object or Array and encoded as MessagePack string type and the encoding attribute is set to "json"
  • (RSL4d3) a JSON Message payload is stringified either as a JSON Object or Array and represented as a JSON string and the encoding attribute is set to "json"

Both branches — MessagePack and JSON — require the same two things: stringification, and encoding: "json".

features.md:311 and :313 (RSL1k1, RSL1k3):

  • (RSL1k1) Idempotent publishing via library-generated Message id s is supported if idempotentRestPublishing (see TO3n) is enabled and one or more Message instances are passed to publish() and all Message s have an empty id attribute. The library generates a base id string by base64-encoding a sequence of at least 9 bytes obtained from a source of randomness. Each individual Message in the set of messages to be published is assigned a unique id of the form <base id>:<serial> (where serial is the zero-based index into the set).
  • (RSL1k3) If more than one Message is passed to publish() and one or more of those messages contains a non-empty id attribute, then all message ids (present or absent) are preserved on sending the batch of messages.

(Emphasis mine on RSL1k1's precondition.)

features.md:1926 (TO3n) and the IDL at :2214:

  • (TO3n) idempotentRestPublishing boolean - defaults to false for clients with version < 1.2, otherwise true. If true, RSL1k applies
      idempotentRestPublishing: bool default true // RSL1k1, RTL6a1, TO3n

features.md:1922 (TO3f):

  • (TO3f) useBinaryProtocol boolean - defaults to true. If false, forces the library to use the JSON encoding for REST and Realtime operations, instead of the default binary msgpack encoding

Defect 1 — channel/publish.md:129: an object payload asserted unstringified

channel/publish.md:76-131, rest/unit/RSL1a/publish-message-array-1. The messages published (:110-114):

messages = [
  Message(name: "event1", data: "data1"),
  Message(name: "event2", data: { "key": "value" }),
  Message(name: "event3", data: bytes([0x01, 0x02, 0x03]))
]

and the assertions (:118-131):

request = captured_requests[0]
body = parse_json(request.body)

ASSERT body.length == 3
ASSERT body[0]["name"] == "event1"
ASSERT body[0]["data"] == "data1"
ASSERT body[1]["name"] == "event2"
ASSERT body[1]["data"] == { "key": "value" }
# Note: binary data encoding tested separately in encoding tests

body[1]["data"] is read directly out of the parsed request body and compared to an object. Under RSL4c3 or RSL4d3 — the test pins neither protocol, but both branches agree — that field is a string, and the message also carries encoding: "json", which this test does not assert.

The decisive evidence is a sibling spec doing the same publish and asserting the opposite. encoding/message_encoding.md:67-108, rest/unit/RSL4b/json-object-encoding-0, publishes data: { "key": "value", "nested": { "a": 1 } } and asserts (:103-107):

request = captured_requests[0]
body = parse_json(request.body)[0]

# Data should be JSON-serialized string
ASSERT body["data"] IS String
ASSERT parse_json(body["data"]) == { "key": "value", "nested": { "a": 1 } }
ASSERT body["encoding"] == "json"

Note the shape of that middle line. Where the corpus means "compare after decoding", it writes parse_json(body["data"]) == {…} explicitly — the same idiom appears at message_encoding.md:239, :961 and :1002. publish.md:129 has neither that wrapper nor an encoding assertion, so it is not a shorthand for the decoded comparison; it asserts the raw field.

Fix

Replace publish.md:129 with the message_encoding.md form:

ASSERT body[1]["data"] IS String
ASSERT parse_json(body[1]["data"]) == { "key": "value" }
ASSERT body[1]["encoding"] == "json"

Defect 2 — channel/publish.md:177: whole-body equality under idempotent publishing

channel/publish.md:135-180, rest/unit/RSL1e/null-name-and-data-0. The table (:163-167):

ID name data Expected body
1 null "hello" [{"data": "hello"}]
2 "event" null [{"name": "event"}]
3 null null [{}]

and the steps (:170-180):

FOR EACH test_case IN test_cases:
  captured_requests = []

  AWAIT channel.publish(name: test_case.name, data: test_case.data)

  body = parse_json(captured_requests[0].body)
  ASSERT body == [test_case.expected_body]
  ASSERT "name" NOT IN body[0] IF test_case.name IS null
  ASSERT "data" NOT IN body[0] IF test_case.data IS null

The client is constructed with a bare key (:157), so idempotentRestPublishing is true per TO3n, and RSL1k1 applies: every message in a single-message publish has an empty id, so the library generates one. The transmitted body is therefore a strict superset of [{"data": "hello"}], and body == [test_case.expected_body] fails for all three cases. Row 3 is the starkest: [{}] cannot be the body of an idempotent publish at all.

channel/idempotency.md asserts both halves of this for the same call shape. At :41:

ASSERT client.options.idempotentRestPublishing == true

and at :77-85, after the identical AWAIT channel.publish(name: "event", data: "data"):

request = captured_requests[0]
body = parse_json(request.body)[0]

ASSERT "id" IN body

So one spec asserts "id" IN body for that publish and the other asserts a body literal with no id in it.

There is no subset-match convention that would rescue the first. grep -rni subset uts/docs/ returns nothing, and the authoring guide pushes the other way — uts/docs/writing-test-specs.md:580:

Use encode_uri_component() for any variable path segment or query parameter in URL assertions. This is defined in the UTS README. Always use exact equality (==) for path assertions, not CONTAINS.

and :1284, in "Common Mistakes to Avoid":

  1. Loose path assertions: ASSERT request.url.path CONTAINS "/channels/" -- Use exact path with encoding

Both are about paths rather than bodies, but they establish that == in this corpus means exact equality, not containment.

Fix

Drop the Expected body column and assert field-wise. The two NOT IN lines already carry RSL1e's actual requirement; what is needed is the positive half:

  body = parse_json(captured_requests[0].body)
  ASSERT body.length == 1
  ASSERT body[0]["name"] == test_case.name IF test_case.name IS NOT null
  ASSERT body[0]["data"] == test_case.data IF test_case.data IS NOT null
  ASSERT "name" NOT IN body[0] IF test_case.name IS null
  ASSERT "data" NOT IN body[0] IF test_case.data IS null

Defect 3 — channel/idempotency.md:409: a generated id in a mixed batch

channel/idempotency.md:360-410, rest/unit/RSL1k/mixed-ids-in-batch-1. The messages (:389-393):

messages = [
  Message(id: "client-id-1", name: "event1", data: "data1"),
  Message(name: "event2", data: "data2"),  # No ID - should be generated
  Message(id: "client-id-2", name: "event3", data: "data3")
]

and the assertions (:402-409):

# Client IDs preserved
ASSERT body[0]["id"] == "client-id-1"
ASSERT body[2]["id"] == "client-id-2"

# Library-generated ID for middle message
ASSERT body[1]["id"] matches pattern "[A-Za-z0-9_-]+:[0-9]+"

RSL1k1 generates ids only where "all Messages have an empty id attribute", which is false here. RSL1k3 then governs and requires that "all message ids (present or absent) are preserved on sending the batch" — so the id-less message must go out with no id. The first two assertions are RSL1k3 correctly applied; the third is its negation.

The "Spec requirement" prose at :364 is the source of the error:

Spec requirement: In a batch publish, messages with client-supplied IDs must be preserved, while messages without IDs receive library-generated IDs using the standard format.

The first clause is RSL1k3. The second clause is traceable to no clause in RSL1k — not RSL1k1 (whose precondition excludes this case), not RSL1k2 (single message with a non-empty id), not RSL1k3 (which says the opposite), and not RSL1k4 or RSL1k5 (both about end-to-end idempotency tests). It appears to have been invented to describe a behaviour no clause requires.

Fix

channel/idempotency.md:409 — replace the pattern match with:

# RSL1k3: an absent id is preserved as absent
ASSERT "id" NOT IN body[1]

and rewrite the prose at :364 to state RSL1k3 alone. This also makes the section a genuine RSL1k3 test, which the file's header (:3) already claims.

Cross-SDK

UTS test The spec ably-pubsub-js ably-python
rest/unit/RSL1a/publish-message-array-1 object payload, ASSERT body[1]["data"] == { "key": "value" } dropped the object payload — publishes 'one', 'two', 'three' and asserts only name on each; no data assertion survives pytest.fail placeholder naming RSL1c/RSL4c3 — there is no spec-correct assertion to write
rest/unit/RSL1e/null-name-and-data-0 ASSERT body == [test_case.expected_body] split into three tests, none doing whole-body equality; asserts expect('name' in body[0]).to.be.false and expect(body[0].data).to.equal('data'); two of the three are gated behind RUN_DEVIATIONS asserts field-wise with an explanatory comment; passes
rest/unit/RSL1k/mixed-ids-in-batch-1 ASSERT body[1]["id"] matches pattern "[A-Za-z0-9_-]+:[0-9]+" inverted the assertion to expect(body[1].id).to.be.undefined, and renamed the test "RSL1k - mixed client and library IDs skips generation" pytest.fail placeholder naming RSL1k1/RSL1k3

Sources: ably/ably-pubsub-js, test/uts/rest/unit/channel/publish.test.ts and test/uts/rest/unit/channel/idempotency.test.ts (fetched via gh api, default branch); ably/ably-python, test/uts/rest/unit/channel/publish_test.py:69, :79 and test/uts/rest/unit/channel/idempotency_test.py:223, with the rationale recorded under "UTS Spec Errors" in test/uts/deviations.md.

On Defect 3 in particular, ably-js did not merely adapt: it asserted the opposite of the spec, retitled the test to describe the opposite behaviour, and recorded nothing. Its comment at the site reads // Second message: no ID generated (allEmptyIds returned false) — the same reasoning as RSL1k1's precondition. Two independent SDKs reached the same reading; only the spec dissents.

A class-wide gap: the protocol is never pinned

This one is not a features.md contradiction, and an earlier version of this claim overreached by treating it as one. It is a question of where a convention is documented.

Eight rest/unit specs read the request body with parse_json(request.body) and never mention useBinaryProtocol:

Spec parse_json(…body) sites useBinaryProtocol
channel/publish.md 8 0
channel/idempotency.md 9 0
channel/annotations.md 5 0
channel/update_delete_message.md 7 0
push/push_admin_publish.md 3 0
push/push_channels.md 2 0
push/push_channel_subscriptions.md 1 0
push/push_device_registrations.md 1 0

encoding/message_encoding.md is the only spec in rest/unit that both reads a body and pins the protocol, and it pins it in both directions, in-line with the option that matters — :132-135:

client = Rest(options: ClientOptions(
  key: "appId.keyId:keySecret",
  useBinaryProtocol: false  # JSON protocol requires base64 for binary
))

and :176-179:

client = Rest(options: ClientOptions(
  key: "appId.keyId:keySecret",
  useBinaryProtocol: true  # MessagePack
))

TO3f (features.md:1922) defaults useBinaryProtocol to true, so on a plain-key client the body is msgpack and parse_json does not apply to it.

uts/docs/integration-testing.md:282 does supply a default:

Spec files without a ## Protocol Variants section default to JSON only. No special handling is required in derived test implementations for these specs.

But that sentence sits in the integration-testing guide, under a ## Protocol Variants mechanism that does not exist in rest/unitgrep -rn 'Protocol Variants' uts/rest/unit/ returns nothing. uts/docs/writing-test-specs.md, which is the document a unit-spec author reads, says nothing about protocol defaults. So the convention that would resolve these eight specs is documented only for a different class of test, and the unit specs use the opposite convention where they address it at all.

The result is that two SDKs read the same eight specs in opposite ways:

  • ably-pubsub-js added useBinaryProtocol: false to every client in all eight files — 92 occurrences, all false, against zero in the specs.
  • ably-python took TO3f at its word and decodes with msgpack in all eight, with comments saying so (test/uts/rest/unit/channel/publish_test.py:30-38: "The body is msgpack, as use_binary_protocol defaults to True. The specification reads it with parse_json"; same at channel/idempotency_test.py:29, channel/annotations_test.py:41, channel/update_delete_message_test.py:35, and in all four push/*_test.py).

Both readings are defensible from the text as written, which is the problem. A test asserting body["encoding"] == "json", or asserting base64 for a binary payload, means different things under the two readings.

The ask

Either of these closes it; the choice is a maintainer's.

  1. Extend the JSON-default convention to unit specs. Add to uts/docs/writing-test-specs.md the same statement integration-testing.md:282 makes — that a spec which does not pin useBinaryProtocol is to be derived against JSON — and say explicitly that a spec whose assertions depend on the encoding must pin it, citing message_encoding.md as the pattern. This is the smaller change and validates what ably-js already did.
  2. Pin the protocol in each of the eight specs. Add useBinaryProtocol: false to the client construction in every section that reads a body, matching message_encoding.md:132-135. This is more edits but leaves nothing implicit, and it is the only option that keeps the specs self-contained.

Whichever is chosen, batch_publish.md needs the same treatment for a related reason: its RSC22c6 section asserts base64 with encoding: "base64" for a binary payload, which is the RSL4d1 JSON-protocol branch, while useBinaryProtocol appears nowhere in the file and TO3f would make RSL4c1 govern.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions