Skip to content

Conversation

@peter-lawrey
Copy link
Member

@peter-lawrey peter-lawrey commented Nov 19, 2025

This PR makes a series of small functional changes around Wire edge-cases and configuration, adds URL-level control for file append semantics, and updates the third-party BOM ro 3.27ea7. It also layers in a lot of documentation and test coverage, but those are secondary to the behaviour changes below.


Functional changes

1. Minor, but real, functional clean-ups

These are smaller behaviour tweaks rather than big features:

  • SelfDescribingTriviallyCopyable

    • Uses the instance description field consistently instead of re-calling $description() when validating and copying; makes the logic more robust if the description is cached or overridden.
  • QueryWire comment/documentation and small refactor

    • Internal renaming (sepseparator) and Javadoc updates with no change to encoded wire format.

    • The tests add coverage for:

      • Percent-encoded characters (value%2Bplus+space) surviving round-trip.
      • Handling of zero bytes and “dangling” keys in query strings.
      • Converting a query string into TextWire-compatible YAML.
  • ServicesTimestampLongConverter

    • Changes longFunction to LongUnaryOperator and keeps the same semantics; the public API (toTime, parse, asString) is unchanged.
    • This is mostly an internal refactor to use JDK functional types.
  • Misc switch default: branches

    • Several switches in BinaryWire, TextWire, YamlWire, etc., now have explicit default: break; branches, making future additions safer and preventing accidental fall-through.

2. Safer reference bindings in BinaryWire and RawWire

Files:

  • BinaryWire.java
  • RawWire.java

Before:

  • Methods such as int32forBinding, int64forBinding, and boolForBinding assumed that the IntValue / LongValue / BooleanValue arguments were of the concrete binary reference types and casted directly:

    ((BinaryIntReference) intValue).bytesStore(bytes, pos, 4);
  • Passing the wrong implementation (e.g. a different IntValue implementation) would cause a ClassCastException at runtime with a fairly unhelpful message.

After:

  • New helper:

    static <T> T requireReference(Object value, Class<T> expected, String action) {
        if (expected.isInstance(value))
            return expected.cast(value);
        throw new IllegalArgumentException(
            action + " requires " + expected.getSimpleName() +
            " but was " + (value == null ? "null" : value.getClass().getName()));
    }
  • BinaryWire.BinaryValueOut now uses requireReference in:

    int32forBinding(int value, @NotNull IntValue intValue)
    int64forBinding(long value, @NotNull LongValue longValue)
    boolForBinding(boolean value, @NotNull BooleanValue booleanValue)
  • RawWire.RawValueOut likewise uses requireReference for the same binding methods.

Impact:

  • Behaviourally unchanged for correct usage (the same concrete reference types continue to work).
  • Incorrect usage now fails fast with a clear IllegalArgumentException instead of a ClassCastException, making misconfigurations easier to diagnose and avoiding obscure errors deep in the binding path.

3. Enforce WriteBytesMarshallable when usesSelfDescribingMessage() is false (BinaryWire)

File: BinaryWire.java

Before:

BinaryLengthLength binaryLengthLength = object.binaryLengthLength();
long pos = binaryLengthLength.initialise(bytes);

if (useSelfDescribingMessage(object))
    object.writeMarshallable(BinaryWire.this);
else
    ((WriteBytesMarshallable) object).writeMarshallable(BinaryWire.this.bytes());

binaryLengthLength.writeLength(bytes, pos, bytes.writePosition());
  • If useSelfDescribingMessage(object) returned false for an object that did not implement WriteBytesMarshallable, this code would throw a ClassCastException at runtime.

After:

BinaryLengthLength binaryLengthLength = object.binaryLengthLength();
long pos = binaryLengthLength.initialise(bytes);

if (useSelfDescribingMessage(object)) {
    object.writeMarshallable(BinaryWire.this);
} else {
    if (!(object instanceof WriteBytesMarshallable))
        throw new InvalidMarshallableException(
            "Object must implement WriteBytesMarshallable when usesSelfDescribingMessage is false: "
            + object.getClass());
    ((WriteBytesMarshallable) object).writeMarshallable(BinaryWire.this.bytes());
}

binaryLengthLength.writeLength(bytes, pos, bytes.writePosition());

Impact:

  • When usesSelfDescribingMessage() is false, objects must genuinely be WriteBytesMarshallable. If not, you now get a clearly classified InvalidMarshallableException with a helpful message.
  • This makes the contract much clearer and avoids silent misuse of the non-self-describing path.

4. Robust bytes(...) copying into arbitrary BytesOut (BinaryWire)

File: BinaryWire.java

Before:

public WireIn bytes(@NotNull BytesOut<?> toBytes, boolean clearBytes) {
    ...
    if (code == U8_ARRAY) {
        ((Bytes) bytes).readWithLength(length - 1, toBytes);
    } else {
        bytes.uncheckedReadSkipBackOne();
        textTo((Bytes) toBytes);   // **unconditional cast to Bytes**
    }
    return wireIn();
}
  • If toBytes was not actually a Bytes instance (e.g. some custom BytesOut), this cast would throw a ClassCastException.

After:

  • A helper that can target any BytesOut:

    private void copyTextToBytesOut(BytesOut<?> target) {
        if (target instanceof Bytes) {
            textTo((Bytes) target);
            return;
        }
        try (ScopedResource<Bytes<Void>> scoped = Wires.acquireBytesScoped()) {
            Bytes<?> temp = scoped.get();
            textTo(temp);
            temp.readPosition(0);
            long length = temp.readRemaining();
            target.write(temp, 0L, length);
            temp.clear();
        }
    }
  • And in bytes(...):

    if (code == U8_ARRAY) {
        bytes.readWithLength(length - 1, toBytes);
    } else {
        bytes.uncheckedReadSkipBackOne();
        copyTextToBytesOut(toBytes);
    }

Impact:

  • BinaryWire now correctly supports non-Bytes BytesOut implementations when copying a textual value.
  • Existing Bytes targets behave exactly as before; only previously broken/non-supported cases now work instead of throwing.

5. DefaultValueIn robustness for defaults and references

File: DefaultValueIn.java

  1. Safer bytesMatch(...) when default isn’t a BytesStore:

    Before:

    Object o = defaultValue;
    BytesStore<?, ?> bytes = (BytesStore) o;   // unchecked cast
    consumer.accept(compareBytes.contentEquals(bytes));
    • If defaultValue wasn’t actually a BytesStore, this would fail with a ClassCastException.

    After:

    Object o = defaultValue;
    if (!(o instanceof BytesStore)) {
        consumer.accept(false);
        return wireIn();
    }
    BytesStore<?, ?> bytes = (BytesStore<?, ?>) o;
    consumer.accept(compareBytes.contentEquals(bytes));

    Impact:

    • Consumers now get false when the default isn’t a BytesStore instead of blowing up; the API contract becomes “no match” rather than “crash”.
  2. Gracefully handling null IntValue / LongValue in reference-binding readers:

    Before:

    value.setValue(o.longValue());
    setter.accept(t, value);   // NPE if value == null

    After:

    LongValue target = value == null ? wireIn.newLongReference() : value;
    target.setValue(o.longValue());
    setter.accept(t, target);

    Similar change for int32(...).

    Impact:

    • Callers can now pass null for the reference argument and still get a valid reference created and passed to the setter. Previously this would crash; now it behaves sensibly.
  3. bool() default check hardened:

    // Before
    return defaultValue == Boolean.TRUE;
    // After
    return Boolean.TRUE.equals(defaultValue);
    • Avoids oddities if defaultValue is a boxed Boolean coming through a different identity.

6. URL ?append= support for FileMarshallableOut

File: FileMarshallableOut.java

Before:

  • Append behaviour was controlled only via the FMOOptions object, loaded through QueryWire from the URL query string. If the query wasn’t in the expected form, append would silently remain false.

After:

  • New parsing for a plain append query parameter:

    final String query = url.getQuery();
    options.append = query != null && parseAppendFlag(query);
    if (query != null) {
        QueryWire queryWire = new QueryWire(Bytes.from(query));
        options.readMarshallable(queryWire);
    }
  • parseAppendFlag(...) decodes the query and looks for append=...:

    private static boolean parseAppendFlag(String query) {
        for (String pair : query.split("&")) {
            if (pair.isEmpty())
                continue;
            int equals = pair.indexOf('=');
            String rawKey = equals >= 0 ? pair.substring(0, equals) : pair;
            String rawValue = equals >= 0 ? pair.substring(equals + 1) : "";
            if ("append".equalsIgnoreCase(decodeComponent(rawKey))) {
                return Boolean.parseBoolean(decodeComponent(rawValue));
            }
        }
        return false;
    }
  • decodeComponent uses URLDecoder with UTF-8 and wraps errors in IORuntimeException.

Impact:

  • Call-sites can now reliably control append vs overwrite semantics with URL query strings like:

    • file:///path/to/out.yaml?append=true
  • Existing QueryWire-style options still work; the explicit parsing only sets a default before reading structured options.


7. Guard MessageHistory.writeHistory against non-WriteDocumentContext

File: MessageHistory.java

Before:

static void writeHistory(DocumentContext dc) {
    if (((WriteDocumentContext) dc).isEmpty()) {
        get().doWriteHistory(dc);
    }
}
  • If the DocumentContext passed in wasn’t a WriteDocumentContext, this would throw a ClassCastException.

After:

static void writeHistory(DocumentContext dc) {
    if (dc instanceof WriteDocumentContext && ((WriteDocumentContext) dc).isEmpty()) {
        get().doWriteHistory(dc);
    }
}

Impact:

  • The method is now safe to call with any DocumentContext implementation; it simply no-ops unless it’s a WriteDocumentContext at the start of a message.
  • Behaviour is unchanged for the existing, correct call-sites.

Non-functional changes (docs, tests, formatting)

While not changing runtime behaviour, these are worth noting:

  • New agent guides:

    • CLAUDE.md and GEMINI.md provide detailed instructions for different AI code tools.
  • Documentation consolidation:

    • Legacy docs/ content moved into src/main/docs/.

    • Large number of .adoc files updated for:

      • Consistent :lang: en-GB, ToC, and highlighter settings.
      • Architecture overview, testing strategy, security review, and requirements traceability.
  • Tests:

    • Many new tests around:

      • DocumentContext rollback (DocumentContextLifecycleTest).
      • Elastic bytes behaviour (ElasticByteBufferTest, ElasticBytesCapacityTest).
      • DefaultValueIn edge cases (DefaultValueInCoverageTest).
      • QueryWire parsing and semantics (QueryWireTest).
      • Method writer/reader edge cases (MethodWriterBytesTest, MethodReaderArgumentsRecycleTest, etc.).
    • These do not change behaviour but lock in the new contracts above.


Behavioural summary

  • No change for normal, correct usage of Wire, BinaryWire, RawWire, DefaultValueIn, and FileMarshallableOut.

  • Improved failures and diagnostics when:

    • Passing the wrong reference implementations into *forBinding(...).
    • Using BinaryWire with usesSelfDescribingMessage == false on a non-WriteBytesMarshallable object.
    • Calling MessageHistory.writeHistory with non-WriteDocumentContexts.
    • Using DefaultValueIn.bytesMatch with non-BytesStore defaults, or null references in int32/int64.
  • New functionality:

    • URL ?append= flag support for FileMarshallableOut.

These changes are mostly hardening/clarification around edge cases and mis-use, plus a dependency BOM bump that may affect downstream behaviour via third-party libraries.

@peter-lawrey peter-lawrey changed the title Improve documentation and formatting across multiple files Documentation consolidation, agent guides, and robustness fixes for Chronicle Wire Nov 19, 2025
@peter-lawrey peter-lawrey changed the title Documentation consolidation, agent guides, and robustness fixes for Chronicle Wire Tighten Wire edge-case handling, add URL append support Nov 20, 2025
@sonarqubecloud
Copy link

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants