Repository navigation
[java, ruby] Group job methods, add updates, and limit output - #1569
Conversation
bd4f506 to
2ae7e6c
Compare
The Java implementation was doing something pretty weird compared to the
others in that job-related functions were all "bare" methods on client
with no prefix:
client.get(...)
client.list(...)
client.cancel(...)
client.delete(...)
client.retry(...)
client.insert(...)
The way this works elsewhere is that they either get a prefix like
`client.JobGet(...)` (Go) or are accessed through a "bundle" like
`client.jobs().get(..)` (Rust).
Java was made even a bit weirder because all its queue-related methods
were already using the "bundle" approach:
client.queues().get(name)
client.queues().list()
client.queues().pause(name)
client.queues().resume(name)
client.queues().update(name, metadata)
Here, take job methods and move them over to a "bundle":
client.jobs().cancel(id)
client.jobs().complete(connection, job)
client.jobs().delete(id)
client.jobs().deleteMany(query, all)
client.jobs().get(id)
client.jobs().list(query)
client.jobs().retry(id)
client.jobs().update(id, options)
The one exception is `insert` (and related methods), which stays as it
was before to match other languages.
One last piece is that we also add a job update method, which didn't
exist before in Java, and which also gives us a way to atomically update
output:
client.jobs().update(jobId, JobUpdateOptions.builder()
.output(Map.of("normalized_text", normalized))
.build());
This is technically breaking of course, but we _just_ released Java so I
think we should go for it. Also, the insert methods aren't affected and
those are going to be by far the most commonly used.
I checked other languages on this convention and Java was the only major
outlier.
2ae7e6c to
2e9b083
Compare
brandur
changed the title
jobs() + add jobs().update()jobs() + add jobs().update()
| @@ -0,0 +1,28 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
🤖 Could we remove the 21 added Eclipse/JDT project files (.project, .classpath, and .settings/*) from this PR? They appear to be generated IDE configuration and are unrelated to the job API change. Shared IDE configuration would be clearer as a separate change.
There was a problem hiding this comment.
🤖 Removed all 21 generated Eclipse/JDT files in 38acec6 and added .project, .classpath, and .settings/ to java/.gitignore. None existed on the PR's base branch. The Java language-server marker and Maven-derived content suggest they were accidentally committed editor artifacts; there was no existing repository convention for them.
| * unchanged. Throws {@link RiverException} with code {@code NOT_FOUND} for a missing job. | ||
| */ | ||
| public Job<JsonNode> update(Connection connection, long id, JobUpdateOptions options) { | ||
| var updates = Objects.requireNonNull(options, "options").metadataUpdates(); |
There was a problem hiding this comment.
🤖 Nonblocking follow-up: this path carries forward Java's missing 32 MiB limit for explicit job output. I reproduced persisting an output value of 34,603,108 encoded UTF-8 bytes through jobs().update() on SQLite; Go, Rust, and JavaScript reject oversized explicit output.
The old Client.output() also omitted this check, so this is an existing Java parity gap. Please track adding consistent output-size validation, ideally shared with WorkContext.output(), with boundary coverage for both supported databases.
There was a problem hiding this comment.
🤖 Fixed in 38acec6. JobUpdateOptions and WorkContext.output() now share validation of encoded UTF-8 JSON against the 32 MiB limit, preserving previous output when validation fails. Checking the other ports also uncovered the same gap in Ruby's Job#output=, which is fixed here too; Go, Rust, and JavaScript already enforce the limit.
Added boundary coverage for one byte below, exactly at, and one byte above the limit, including multibyte UTF-8 and JSON escaping. The tests exercise Postgres and SQLite; Ruby's shared tests cover both Active Record and Sequel. Both Java database suites and Java lint passed, as did the full Ruby suite (3,060 examples) and Ruby lint.
brandur
changed the title
jobs() + add jobs().update()
🤖 Java's job methods currently sit directly on
Client, while queue methods live underclient.queues(). Move job reads, cancellation, completion, deletion, and retries toclient.jobs()for consistent organization. This is a breaking API change;insertandinsertManykeep their existing API.Add
client.jobs().updatewith immutableJobUpdateOptionsfor immediate output writes and top-level metadata merges, replacingClient.output(). Connection-first overloads use the caller's transaction.Explicit output is limited to 32 MiB of encoded UTF-8 JSON, with shared validation for
JobUpdateOptionsandWorkContext.output(). Apply the same limit to Ruby'sJob#output=to close its matching parity gap. Invalid output preserves the previous value. Go, Rust, and JavaScript already enforce the limit.Ignore generated Eclipse/JDT project files. Update API documentation, examples, conformance callers, and tests for the new job API.
Validation:
make test/java/sqlite MVN='mvn clean'make test/java/postgres MVN='mvn clean'with the local test databasemake -C java lint MVN='mvn clean'make -C ruby test TEST_JOBS=1with both databases required: 3,060 examples passedmake -C ruby lintBoundary tests cover one byte below, exactly at, and one byte above the limit, multibyte UTF-8, JSON escaping, and preservation of previous output on both supported databases. Ruby's shared tests cover both Active Record and Sequel.