Skip to content

Commit 603700c

Browse files
dfa1claude
andcommitted
refactor: drop ServiceLoader SPI from ReadRegistry and WriteRegistry (#255)
Both registries are now builder-only, consistent with LayoutRegistry. Built-in decoders/encoders are registered explicitly via registerDefaults() instead of META-INF/services discovery. loadAll() delegates to builder().registerDefaults().build(). Also adds VarBinArrayTest coverage for all codes-ptype branches in forEachI32OffsetByteLength (U8/U16/U32/I64/U64 + unsupported-ptype throw), fixing the Sonar new_coverage gate. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 84090d0 commit 603700c

28 files changed

Lines changed: 291 additions & 188 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Changed
11+
12+
- `ReadRegistry` and `WriteRegistry` are now builder-only (no `ServiceLoader`), matching `LayoutRegistry`. `registerServiceLoaded()` is replaced by `registerDefaults()`, which registers all built-in encoders/decoders explicitly; `loadAll()` is unchanged. `ReadRegistry.decoderMap()` is added. Custom encodings register via `register(...)` on the builder instead of a `META-INF/services` file. ([#255](https://github.com/dfa1/vortex-java/issues/255))
13+
1014
### Fixed
1115

1216
- `VarBinArray.DictMode` reads dict-value offsets in a ptype-uniform loop: the I32 fast path eliminates the per-row `switch(dictValOffPType)` that blocked C2 vectorization (introduced by #215). ([#243](https://github.com/dfa1/vortex-java/issues/243))

CLAUDE.md

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -124,25 +124,25 @@ Layout tree: `Struct → Zoned(Stats) → Chunked → [Flat, Flat, ...]`
124124
- **Zoned** (`vortex.stats`; reads also accept the newer Rust alias `vortex.zoned`) wraps a child with per-chunk min/max for zone-map pruning
125125

126126
Encoding IDs are strings (`"vortex.primitive"`, `"fastlanes.bitpacked"`). `ReadRegistry` maps IDs →
127-
`EncodingDecoder` via `ServiceLoader`; immutable after construction — register custom decoders on
128-
the builder: `ReadRegistry.builder().registerServiceLoaded().register(myDecoder).build()`.
127+
`EncodingDecoder`; immutable after construction, built-in decoders are registered explicitly by
128+
`registerDefaults()` — register custom decoders on the builder:
129+
`ReadRegistry.builder().registerDefaults().register(myDecoder).build()`.
129130

130131
### Adding an encoding
131132

132133
Add an `EncodingId.WellKnown` constant `VORTEX_FOO("vortex.foo")` (re-exported on the interface), then per side:
133-
- **Decode:** `FooEncodingDecoder implements EncodingDecoder` in `reader.decode` + FQN in
134-
`reader/.../META-INF/services/io.github.dfa1.vortex.reader.decode.EncodingDecoder`
135-
- **Encode:** `FooEncodingEncoder implements EncodingEncoder` in `writer.encode` + FQN in
136-
`writer/.../META-INF/services/io.github.dfa1.vortex.writer.encode.EncodingEncoder`
134+
- **Decode:** `FooEncodingDecoder implements EncodingDecoder` in `reader.decode` + a
135+
`.register(new FooEncodingDecoder())` call in `ReadRegistry.Builder#registerDefaults()`
136+
- **Encode:** `FooEncodingEncoder implements EncodingEncoder` in `writer.encode` + a
137+
`.register(new FooEncodingEncoder())` call in `WriteRegistry.Builder#registerDefaults()`
137138

138139
### Adding an extension type
139140

140141
Add `ExtensionId` constant, then per side:
141142
- **Decode:** singleton `FooExtensionDecoder implements ExtensionDecoder` in `reader.extension` +
142-
a `case VORTEX_FOO` in `Chunk.as()` — not registry-managed, **no service file**
143-
(`registerServiceLoaded()` only discovers `EncodingDecoder`).
144-
- **Encode:** `FooExtensionEncoder implements ExtensionEncoder` in `writer` + FQN in
145-
`writer/.../META-INF/services/io.github.dfa1.vortex.writer.ExtensionEncoder`
143+
a `case VORTEX_FOO` in `Chunk.as()` — not registry-managed.
144+
- **Encode:** `FooExtensionEncoder implements ExtensionEncoder` in `writer` + a
145+
`.register(FooExtensionEncoder.INSTANCE)` call in `WriteRegistry.Builder#registerDefaults()`
146146

147147
## Memory model
148148

@@ -204,17 +204,18 @@ When stuck on encode/decode behavior, consult **in this order**:
204204

205205
- **DType is pluggable only via `Extension`.** `DType` is a sealed interface; downstream code must
206206
not add variants. Use `new DType.Extension("ip.address", new DType.Primitive(PType.I32, false),
207-
null, false)` and register decoders/encoders on the registries (or `ServiceLoader<ExtensionEncoder>`).
207+
null, false)` and register decoders/encoders on the registries.
208208
Mirrors Rust (`vortex.date`, `vortex.uuid`, …). No SPI for DType variants planned.
209-
- **Layout decode is pluggable via `LayoutDecoder` + `LayoutRegistry`** (`reader.layout`) — the
210-
Rust reference registers layouts at runtime, so ours are open too. Builder-registered only
211-
(`LayoutRegistry.builder().registerDefaults().register(custom).build()`, pass to
212-
`VortexReader.open(path, readRegistry, layoutRegistry)`) — **no service file**, by decision:
213-
Rust registers layouts explicitly on the session (no auto-discovery exists there), and a
214-
classpath jar must not silently change scan traversal — layout registration stays visible at
215-
the open() call site. Encodings keep ServiceLoader because they are leaf codecs with a
216-
plausible drop-in-jar ecosystem. Unknown layouts
217-
fail loudly (`VortexException`, Rust default; no allowUnknown for layouts). Scope: the SPI covers
209+
- **Encoding and layout decode are both pluggable via builder-only registries** — no
210+
`ServiceLoader`. Encodings register on `ReadRegistry`/`WriteRegistry`
211+
(`ReadRegistry.builder().registerDefaults().register(custom).build()`); layouts register on
212+
`LayoutRegistry` (`LayoutRegistry.builder().registerDefaults().register(custom).build()`, pass to
213+
`VortexReader.open(path, readRegistry, layoutRegistry)`). Builder-registered only, by decision:
214+
the Rust reference registers encodings and layouts explicitly on the session (no auto-discovery
215+
exists there), and a classpath jar must not silently change decode/scan behavior — registration
216+
stays visible at the `open()`/`create()` call site. Unknown encodings can be opted into an
217+
allow-unknown passthrough (`ReadRegistry.Builder#allowUnknown()`); unknown layouts always fail
218+
loudly (`VortexException`, Rust default; no allowUnknown for layouts). Scope: the layout SPI covers
218219
full-column subtree decode; zone-map pruning, filtered scans, and chunk planning recognize the
219220
built-in layouts only.
220221
- **Small public APIs.** Don't expose internals — when in doubt, leave it out or make it private.

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ line-by-line diff is zero.
6666
<dependency>
6767
<groupId>io.github.dfa1.vortex</groupId>
6868
<artifactId>vortex-reader</artifactId>
69-
<version>0.11.0</version>
69+
<version>0.12.1</version>
7070
</dependency>
7171
```
7272

@@ -99,7 +99,7 @@ try (VortexReader vf = VortexReader.open(Path.of("data/example.vortex"));
9999
<dependency>
100100
<groupId>io.github.dfa1.vortex</groupId>
101101
<artifactId>vortex-writer</artifactId>
102-
<version>0.11.0</version>
102+
<version>0.12.1</version>
103103
</dependency>
104104
```
105105

calcite/src/test/java/io/github/dfa1/vortex/calcite/UnsignedColumnTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ private static WriteOptions noZoneMaps() {
114114
}
115115

116116
private static ReadRegistry registry() {
117-
return ReadRegistry.builder().registerServiceLoaded().build();
117+
return ReadRegistry.builder().registerDefaults().build();
118118
}
119119

120120
private static List<Object[]> drain(VortexTable table) {

calcite/src/test/java/io/github/dfa1/vortex/calcite/VortexAdapterCoverageTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ static void write() throws Exception {
7474
}
7575

7676
private static ReadRegistry registry() {
77-
return ReadRegistry.builder().registerServiceLoaded().build();
77+
return ReadRegistry.builder().registerDefaults().build();
7878
}
7979

8080
@Test

docs/compatibility.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@ A consumer that only needs to read Vortex files can depend on a strict subset:
1212
<dependency>
1313
<groupId>io.github.dfa1.vortex</groupId>
1414
<artifactId>vortex-reader</artifactId>
15-
<version>0.11.0</version>
15+
<version>0.12.1</version>
1616
</dependency>
1717

1818
<!-- optional: inspector for layout-tree introspection -->
1919
<dependency>
2020
<groupId>io.github.dfa1.vortex</groupId>
2121
<artifactId>vortex-inspector</artifactId>
22-
<version>0.11.0</version>
22+
<version>0.12.1</version>
2323
</dependency>
2424
```
2525

@@ -29,8 +29,8 @@ pinned by the vortex BOM); without them, touching a `vortex.zstd` segment fails
2929
`VortexException` that names the two artifacts. All other encodings are pure Java.
3030

3131
`./mvnw -pl core,reader,inspector verify` builds the read-only artifact set
32-
without the writer module on the classpath. `ServiceLoader<EncodingDecoder>`
33-
resolves only the standalone decoders in `reader`; no encoder class is loaded.
32+
without the writer module on the classpath. `ReadRegistry.loadAll()` registers
33+
only the built-in decoders in `reader`; no encoder class is loaded.
3434

3535
## Known wire-format gaps
3636

@@ -172,7 +172,7 @@ passthrough mode to read such files without failing:
172172

173173
```java
174174
ReadRegistry registry = ReadRegistry.builder()
175-
.registerServiceLoaded()
175+
.registerDefaults()
176176
.allowUnknown()
177177
.build();
178178
try (VortexReader vf = VortexReader.open(path, registry)) {

docs/explanation.md

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -403,12 +403,11 @@ writer.close()
403403

404404
### How `ReadRegistry` resolves encodings
405405

406-
`ReadRegistry.loadAll()` uses `ServiceLoader` to discover all `EncodingDecoder`
407-
implementations on the classpath. Each decoder declares its identity via `encodingId()`.
408-
At decode time the registry maps the typed id from the array node to the right
409-
`EncodingDecoder` instance and calls `decode(DecodeContext)`.
406+
`ReadRegistry.loadAll()` registers all built-in `EncodingDecoder` implementations. Each
407+
decoder declares its identity via `encodingId()`. At decode time the registry maps the typed
408+
id from the array node to the right `EncodingDecoder` instance and calls `decode(DecodeContext)`.
410409

411-
Custom decoders can be added at build time: `ReadRegistry.builder().registerServiceLoaded().register(myDecoder).build()`.
410+
Custom decoders can be added at build time: `ReadRegistry.builder().registerDefaults().register(myDecoder).build()`.
412411
Files with unrecognized IDs throw `VortexException` unless the builder enabled `allowUnknown()`.
413412

414413
## Testing strategy

docs/how-to.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ import io.github.dfa1.vortex.reader.ReadRegistry;
304304
import io.github.dfa1.vortex.reader.array.UnknownArray;
305305

306306
ReadRegistry registry = ReadRegistry.builder()
307-
.registerServiceLoaded()
307+
.registerDefaults()
308308
.allowUnknown()
309309
.build();
310310

docs/reference.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ Immutable after construction. Build via `ReadRegistry.builder()` or the static c
209209
| Method | Notes |
210210
|-----------------------------|------------------------------------------------------------------------------|
211211
| `static builder()` | Returns a fresh `Builder` |
212-
| `static loadAll()` | Immutable registry populated via `ServiceLoader` |
212+
| `static loadAll()` | Immutable registry populated with all built-in decoders |
213213
| `static empty()` | Immutable empty registry (strict mode) |
214214
| `hasDecoder(EncodingId)` | Lookup |
215215
| `isAllowUnknown()` | Predicate |
@@ -219,12 +219,12 @@ Immutable after construction. Build via `ReadRegistry.builder()` or the static c
219219
| Method | Notes |
220220
|------------------------------|------------------------------------------------------------------------------------------|
221221
| `register(EncodingDecoder)` | Add a custom encoding decoder; throws if already registered |
222-
| `registerServiceLoaded()` | Add every `EncodingDecoder` discovered via `ServiceLoader` |
222+
| `registerDefaults()` | Add every built-in `EncodingDecoder` |
223223
| `allowUnknown()` | Switch to passthrough mode — unknown nodes (and their children) decode as `UnknownArray` |
224224
| `build()` | Produce the immutable `ReadRegistry` |
225225

226-
Register custom encoding decoders via `ServiceLoader` by adding the fully qualified class name to
227-
`META-INF/services/io.github.dfa1.vortex.reader.decode.EncodingDecoder`. Extension decoders
226+
Register custom encoding decoders programmatically via `register(EncodingDecoder)` — there is no
227+
`ServiceLoader` discovery. Extension decoders
228228
(`io.github.dfa1.vortex.reader.extension.ExtensionDecoder`) are not registry-managed: the built-in
229229
implementations are singletons invoked directly by their `ExtensionId`.
230230

docs/testing.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,8 @@ Two [architectural fitness functions](https://www.thoughtworks.com/insights/book
190190
- `DocsConsistencyTest`**accuracy**: FQNs resolve, method claims name real methods,
191191
`META-INF/services` mentions exist, relative links don't dangle.
192192
- `EncodingTableFitnessTest`**completeness**: the encodings table in `docs/compatibility.md`
193-
lists exactly the encodings the code registers via `ServiceLoader` (id, decoder/encoder class,
194-
✅/❌ flags), asserted not generated — on drift it prints the exact rows to add.
193+
lists exactly the encodings the code registers in `ReadRegistry`/`WriteRegistry` (id,
194+
decoder/encoder class, ✅/❌ flags), asserted not generated — on drift it prints the exact rows to add.
195195

196196
## Docs consistency gate
197197

0 commit comments

Comments
 (0)