Repository navigation
Commit 6b9e6df
fix(service-datasource): Setup → Datasources: Test answers truthfully for the primary datasource and for drivers that cannot introspect; Sync on the primary datasource refuses with the true reason (#24852)
Fixes #24772
Clause-②: yes (widening)
## What was wrong
Setup → Datasources listed the primary datasource as `default · sqlite ·
connected`, and that is true. `DefaultDatasourcePlugin` connects it
through `DatasourceConnectionService` with `asDefault`, and ADR-0062
registers its driver under the driver's own name
(`com.objectstack.driver.sql`), not under `default`. Test and Sync then
introspected by the name `default`. The engine's registry has no entry
by that name, so both answered `[ObjectQL] Datasource 'default' has no
registered driver to introspect.`: Test as `200 { ok: false }`, Sync as
`400 EXTERNAL_DATASOURCE_ERROR`. A connected `memory` datasource got the
same `ok: false` from Test, because Test was an introspection round trip
and that driver has no `introspectSchema`.
Measured before the change, on `origin/main` `c3a8e07b6`, through the
real admin routes over a real `ObjectQL` engine, the real connection
service and the real `ExternalDatasourceServicePlugin`:
```text
POST /api/v1/datasources/default/test -> 200 {"ok":false,"error":"[ObjectQL] Datasource 'default' has no registered driver to introspect."}
GET /api/v1/datasources/default/remote-tables -> 400 EXTERNAL_DATASOURCE_ERROR, same message
POST /api/v1/datasources/qa_ram/test (memory) -> 200 {"ok":false,"error":"[ObjectQL] Driver for datasource 'qa_ram' does not support introspectSchema()."}
admin list -> default status "ok", qa_ram status "ok"
```
## What changed (route D, as the seat ruled, comment 6287770114)
All of it is in `packages/services/service-datasource/src`. Nothing in
`packages/objectql`.
- **One resolver, reused.** `registeredDriverName()` in
`datasource-connection-service.ts` is the default-driver resolver
`DatasourceConnectionService.disconnect()` applied inline. It is
extracted, and `disconnect()` calls it with its own `asDefault` flag, so
its behaviour is unchanged. The Test path calls the same function.
`disconnectAll()` names the reserved datasource through the same
constant.
- **Test, `POST /api/v1/datasources/:name/test`.** The plugin wires a
new optional `liveDriver` seam: the engine's registry entry under the
name `registeredDriverName()` resolves. `testConnection()` then:
- for the primary datasource, answers the driver's `checkHealth()`,
giving `{ ok: true, latencyMs }`, and does not introspect the platform
database;
- for a driver with no `introspectSchema` (`memory`, `mongodb`), answers
`checkHealth()` with the answer's existing keys, `{ ok: true, latencyMs
}` and no `tableCount`, instead of `ok: false`;
- for a registered driver whose health check fails, answers `ok: false`
with a reason;
- for every other datasource, and for one with no live driver, runs the
same introspection round trip as before, with the same `tableCount` and
the same engine message.
- **Sync on the primary datasource.** `listRemoteTables`,
`generateObjectDraft`, `importObject` and `refreshCatalog` refuse
`default` before any introspection. The status and code are the ones
each door served before: `400 EXTERNAL_DATASOURCE_ERROR`, or
`EXTERNAL_IMPORT_ERROR` on the import. The message is now the true
reason: `Datasource 'default' is the platform's managed primary
datasource, not a federation source: its tables are the platform's own
objects, so there are no remote tables to list, draft or import from it.
Sync objects from an external datasource instead.`
## The measured choice for Test on `default`: `checkHealth()`, not
introspection by the resolved name
Both reach the live driver. `checkHealth()` was chosen for three
reasons:
- Introspection costs a round trip per table of the platform database.
Measured with the real `SqlDriver` on SQLite in memory:
`introspectSchema()` took 14.0 ms at 10 tables and 132.0 ms at 120
tables. `checkHealth()` took 0.79 ms and 0.28 ms. On a networked
Postgres, each of those per-table round trips also pays the network
latency.
- The connection question is what Test asks, and `checkHealth()` is that
question (`SELECT 1` on SQL, `ping` on MongoDB).
- The primary datasource is not a federation source, so a `tableCount`
of the platform's tables answers nothing the page uses. No consumer in
either repository reads `tableCount`.
Every other datasource keeps the introspection probe and its
`tableCount`, so nothing moves for a federation source.
## The seat's premises, re-checked before editing
- **P1, holds.** The resolver `disconnect()` applied (`opts.asDefault ?
engine.getDefaultDriverName() : name`) needs only `getDefaultDriverName`
and `getDriverByName`. Both are on the `'data'` engine's published
contract (`IDataEngine`, `packages/spec/src/contracts/data-engine.ts`),
which the plugin already reads. No objectql change is needed.
- **P2, holds.** The Setup driver catalog (`driver-catalog.ts`) lists
`memory`, `sqlite`, `postgres`, `mysql` and `mongodb`. The two without
`introspectSchema` have `checkHealth()`: `MemoryDriver.checkHealth()`
returns `true`, and `MongoDBDriver.checkHealth()` runs `db.command({
ping: 1 })`. `IDataDriver` declares `checkHealth()` as required.
## Evidence after the change
`packages/services/service-datasource/src/__tests__/primary-datasource-test-and-sync.test.ts`
has 10 cases. They run through the real composition: a real `ObjectQL`
engine (its source, through this package's vitest alias), the connection
service connecting `default` with `asDefault` exactly as
`DefaultDatasourcePlugin.init()` does, the plugin's real wiring, and the
admin routes over Hono.
- On `default`, the list reads `ok`, and Test answers `200 { ok: true,
latencyMs }` with no `error` and no `tableCount`. The driver's
`introspectSchema` is never called.
- On `default`, `remote-tables` and `object-draft` answer `400`, with
`error.code` `EXTERNAL_DATASOURCE_ERROR` and the true message. The
import refuses with `{ code: 'EXTERNAL_IMPORT_ERROR', status: 400 }`,
and the list and catalog refresh with `{ code:
'EXTERNAL_DATASOURCE_ERROR', status: 400 }`. The platform database is
never introspected.
- A `memory` datasource tests `{ ok: true, latencyMs }`, exactly those
two keys. A registered driver whose health check answers `false` or
throws gives `ok: false`.
- Preservation: a managed runtime SQLite datasource still tests `{ ok:
true, tableCount: 1 }` and still lists its tables with `200`. That is
the seat's ban on changing Sync for other managed datasources. Sync on
`memory` still refuses with the engine's reason. A datasource with no
live driver still answers `ok: false`.
Ablations, one behaviour each. Each was run through
`scripts/ablation-replace.mjs`: the anchor hit once, the on-disk blob
changed, and the restore was proven blob == HEAD with an empty `git diff
HEAD`. The subject is this package's own source, imported relatively, so
no build sits between the mutation and the reading.
| leg | mutation | reading |
|:---|:---|:---|
| A1 | the Test path looks the driver up by the datasource name, not
through `registeredDriverName` | red, 1 failed: `expected { ok: false,
…(1) } to match object { ok: true }` |
| A2 | route A put back: no primary guard on `listRemoteTables`, and
introspection resolves the default | red, 2 failed: `expected 200 to be
400`, and `listRemoteTables('default')` resolved a value |
| A3 | a driver without `introspectSchema` goes back to the
introspection round trip | red, 2 failed: `expected { ok: false, …(1) }
to match object { ok: true }` (re-run at `3226d4019`) |
| A4 | Test on `default` introspects by the resolved name | red, 1
failed: `expected [ 'latencyMs', 'ok', 'tableCount' ] to deeply equal [
'latencyMs', 'ok' ]` (re-run at `3226d4019`) |
| A5 | a failed health check is ignored | red, 1 failed: `expected true
to be false` |
| A6 | the dropped `introspection` key put back on a driver that cannot
introspect | red, 1 failed: `expected [ 'introspection', 'latencyMs',
'ok' ] to deeply equal [ 'latencyMs', 'ok' ]` (at `3226d4019`) |
## The published-surface addition: Clause-② yes (widening)
`ExternalDatasourceServiceConfig` is exported from the package's `.`
entry, so its new optional member `liveDriver` widens the published
surface. The changeset is a `minor` bump that declares `Clause-②: yes
(widening)` and names that member. No status code, error code or
accepted value moves on any route. Test's answer keeps its existing keys
(`ok`, `latencyMs`, `tableCount`, `error`): the `introspection` key from
the first head was dropped, because nothing reads it.
- `ExternalDatasourceServiceConfig.liveDriver?`: a host that builds the
service without it keeps the old probe.
`registeredDriverName` and `DEFAULT_DATASOURCE_NAME` are module exports
only, not exported from the package entry.
## Acceptance notes
- **`refreshCatalog` is included under the bounded in-place exemption.**
It is the same defect class (the same false "no registered driver"
answer for `default`), in the same file, through the same refusal
helper. Its door (`POST …/external/refresh-catalog`) keeps `400
EXTERNAL_DATASOURCE_ERROR`.
- **A boundary, checked by reading the code.** The refusal is keyed on
the host-reserved name `default` (ADR-0062). The only way to register a
driver literally named `default` is `ObjectQL.create({ datasources: {
default: driverWithNoName } })`. A repository grep finds no production
caller of `ObjectQL.create`, and every in-tree driver carries its own
name. So no in-tree composition answered `200` on Sync for `default`
before this change.
- **The card's "no edit or delete" is by design, not a defect.** The
tester's own `38-integrations-datasources-created.png` shows Edit and
Delete on the runtime `qa_mem` row. Only the code-origin `default` row
lacks them: code-defined datasources are read-only
(`datasource-admin-service.ts` header), and objectui hides both on
`origin: 'code'`.
- **Not in this PR, per the ruling.** `ObjectQL.introspectDatasource`
still does not apply ADR-0062 (no objectql edit). Sync on other managed
runtime datasources is unchanged (route B, filed separately by the
seat). The two objectui renderer findings are filed by the seat.
- **Noted, not filed: the draft probe discards the answer of
`checkHealth()`.** `DatasourceAdminServicePlugin.probe`, behind `POST
/api/v1/datasources/test`, awaits `driver.checkHealth()` and ignores the
boolean it returns, so a draft whose driver's health check answers
`false` would report `ok: true`. This comes from reading the code only:
no public-door reading was taken.
## Verification
At HEAD `3226d4019` (no further merge of `origin/main`: no conflict and
no main-caused red, AGENTS.md §10):
- `pnpm --filter '@objectstack/service-datasource^...' build`: exit 0.
- `pnpm --filter @objectstack/service-datasource build`: exit 0, and
`check-dts-emitted` reports 4/4.
- `pnpm --filter @objectstack/service-datasource typecheck`: exit 0.
`--listFiles` includes the new test file.
- `pnpm --filter @objectstack/service-datasource test`: 48 files, 862
tests passed, 47.6 s.
- Gate families from `node scripts/pm/dispatch-gates.mjs --commands
--repo objectstack-ai/objectstack` (69 commands): 68 exited 0 and 1 was
not measured. `pnpm check:dual-build-cjs-loads` exited 3 (PREREQUISITE
NOT MET: it reads every package's built output, and this worktree builds
only this package's closure), so CI's `Lint & Repo Gates` is its
reading. `--ran` reconciliation: `69 derived, 68 run, 1 NOT-MEASURED, 0
UNRUN`, with an exit code on every line. `node
scripts/check-platform-checklist.mjs` is not owed: no exported symbol is
renamed or deleted.
`test:repo` was not run locally. CI owns it, and this diff touches no
test of that project.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 81120ca commit 6b9e6df
5 files changed
Lines changed: 451 additions & 3 deletions
File tree
- .changeset
- packages/services/service-datasource/src
- __tests__
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
219 | 219 | | |
220 | 220 | | |
221 | 221 | | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
222 | 254 | | |
223 | 255 | | |
224 | 256 | | |
| |||
714 | 746 | | |
715 | 747 | | |
716 | 748 | | |
717 | | - | |
| 749 | + | |
718 | 750 | | |
719 | 751 | | |
720 | 752 | | |
| |||
883 | 915 | | |
884 | 916 | | |
885 | 917 | | |
886 | | - | |
| 918 | + | |
887 | 919 | | |
888 | 920 | | |
889 | 921 | | |
| |||
0 commit comments