Skip to content

Fix unique-by-args handling of literal JSON keys - #1387

Open
bgentry wants to merge 2 commits into
masterfrom
bg/unique-args-escape-paths
Open

bgentry wants to merge 2 commits into
masterfrom
bg/unique-args-escape-paths

Conversation

@bgentry

@bgentry bgentry commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

This fixes UniqueOpts.ByArgs incorrectly skipping distinct jobs when their JSON field names contain dots or other characters that River interprets as path syntax.

A dot is allowed in a JSON key. This object has one top-level key named user.id:

{"user.id": "u1"}

River's uniqueness code mistakenly reads that key name as a path to an id field inside a user object, as though the args had this different shape:

{"user": {"id": "u1"}}

The lookup finds nothing in the actual args, so the user ID is left out of the unique key. Jobs for different users then look like duplicates.

An application can explicitly choose the dotted JSON name with a struct tag:

type SyncUserArgs struct {
    UserID string `json:"user.id" river:"unique"`
}

func (SyncUserArgs) Kind() string { return "sync_user" }

encoding/json writes the first JSON shape above. It does not turn the dot into nesting, and River does not automatically rename UserID to user.id. The example uses an unusual but valid JSON name; a conventional json:"user_id" tag is unaffected.

With UniqueOpts{ByArgs: true}, these successive insertions now behave correctly. Assume no matching jobs exist initially and the inserted jobs remain available:

Args Before After
SyncUserArgs{UserID: "u1"} Inserted Inserted
SyncUserArgs{UserID: "u2"} Skipped as a duplicate of u1 Inserted
SyncUserArgs{UserID: "u1"} again Skipped as a duplicate Skipped as a duplicate of u1

The fix also covers uniqueness based on all args, without river:"unique" tags. For example, {"file.name":"a.txt"} and {"file.name":"b.txt"} previously collided; now both jobs are inserted. An empty key such as {"":"a"} previously failed with path cannot be empty; now it inserts normally, and changing its value produces a distinct job. Keys containing other path syntax, such as alice@example.com or a leading :, are also treated literally.

A related fix handles unnamed JSON tags, such as this field:

Recipient string `json:",omitempty" river:"unique"`

Go uses the JSON key Recipient, but River previously looked for an empty key, so different nonempty recipients collided. River now uses the Go field name too. An omitted recipient remains omitted.

Internally, tagged fields use escaped gjson/sjson path components. When all args participate in uniqueness, River walks the top-level object directly and rebuilds it in key order using its raw values.

Unaffected args keep byte-for-byte identical unique keys. Affected jobs receive corrected keys, so a job inserted before upgrading may no longer deduplicate an identical insertion after upgrading; during a rolling upgrade, old and new clients may each insert it.

@bgentry
bgentry force-pushed the bg/unique-args-escape-paths branch from 24cabfb to 3a2c0a1 Compare September 25, 2026 00:30
Comment thread internal/dbunique/db_unique.go Fixed
@bgentry
bgentry force-pushed the bg/unique-args-escape-paths branch from 3a2c0a1 to 47f0cc1 Compare September 25, 2026 00:58
@bgentry bgentry changed the title Fix unique-by-args deduplication for keys containing JSON path syntax Fix unique-by-args handling of literal JSON keys Sep 25, 2026
@bgentry
bgentry marked this pull request as ready for review September 25, 2026 01:03
Fields marked `river:"unique"` are read and written as JSON paths, so a
name like `user.id` can omit its value and deduplicate distinct jobs.
Unnamed JSON tags also omit their fields instead of using the Go field
name.

Escape each path component, including leading colons, and fall back to
the Go field name for unnamed tags. Keep the original field ordering so
unaffected jobs retain their existing unique keys.

Cover escaped components, literal and nested path collisions, and unnamed
tags with explicit encoding fixtures and cross-driver insertion cases.
The all-args unique key builder interprets object keys as JSON paths.
Keys containing path syntax can lose their values or collide with other
keys, while an empty key causes an insertion error.

Walk the object directly and rebuild it in key order with raw values.
Preserve the previous key encoding and first-value handling for repeated
keys so unaffected hashes remain stable. Continue rejecting scalar and
nonempty array args instead of silently hashing them as empty objects.

Cover literal keys and duplicate detection across drivers. Pin the key
encoding with explicit compatibility fixtures, including Unicode, control
characters, and raw nested values, and verify rejection of non-object args.
@bgentry
bgentry force-pushed the bg/unique-args-escape-paths branch from 47f0cc1 to b5ceebd Compare September 25, 2026 15:29
@bgentry
bgentry requested a review from brandur September 25, 2026 15:44
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.

2 participants