Create openconnector.csr - #1
Merged
Merged
Conversation
rubenvdlinde
added a commit
that referenced
this pull request
May 21, 2026
Single biggest category was the same DI bug in 7 places: controllers called `ObjectService::find()` and checked the result for `null`, but that method *throws* `OCP\AppFramework\Db\DoesNotExistException` on a missing UUID — the null check was dead code and the exception bubbled up as a 500. Wrapped each call in try/catch returning 404: - SourcesController::test - JobsController::run, JobsController::test - SynchronizationsController::test, ::run, ::deleteLog - SynchronizationContractsController::activate, ::deactivate, ::execute - EventsController::messages, ::pull, ::unsubscribe, ::subscriptionMessages Other fixes: - EndpointsController::logs — was hard-coded to return 500 with "Endpoint logging is not available at this time". Returning a 200 empty paginated result instead so polling clients don't break; real result set lands when the OR call_log integration is wired. - MappingsController::saveObject — was calling OR's saveObject with the OLD positional signature `(register, schema, object)`; new sig is `(object, register?, schema?)`. Passing the register slug as arg #1 surfaced as a TypeError → 500. Rewired with named args. - EventsController subscription routes — `int $subscriptionId` signature cast `"abc-uuid"` to `0`, then find() reported "not found" 404 for any caller. Switched to `string $subscriptionId` across pull, unsubscribe, updateSubscription, subscriptionMessages. - appinfo/routes.php — disabled the `dso#receiveVerzoek` route. The `DsoController` class doesn't exist; the route was an orphan from before the OR cutover. Re-enable once the controller is restored. - Newman collection — corrected several fixture body shapes that the schema validator rejects: * rule fixture: added required `action` + `order` * event fixture: added required `type` (was `eventType`) * source PUT update: full-replace requires all required props * mapping `/mappings/test`: `inputObject` not `input`, plus the mapping body needs `{mapping: {...}}` envelope * mapping `/mappings/objects`: needs `{register, schema, object}`, not flat `{name, payload}` * event_subscription: `style:pull` + `types:[]` + `protocol:HTTP` (NOT `sink:pull` + `eventTypes`) — the spec uses style/types everywhere downstream * dropped the `12 — DSO` folder along with the orphan route Local: 94/94 assertions pass, 19s total run, 191ms avg response.
SudoThijn
added a commit
that referenced
this pull request
Aug 5, 2026
…sisted
POST /api/synchronizations/{id}/test answered 500 with "findContract():
Argument #1 ($id) must be of type string|int, null given" for every
first-ever dry run.
`result['contracts']` is a sparse list: processSynchronizationObject()
pushes $contractUuid unconditionally and leaves it null whenever the
contract carries no uuid. In a dry run that is the normal case rather than
an edge case — synchronizeContract() returns the contract in-memory without
persisting it (REQ-011, the no-write guarantee), so a contract that did not
already exist has no uuid at all and every entry is null. The `_embed`
enrichment then mapped all of them through findContract(), which is typed
`string|int`, so the null was a TypeError escaping the whole run, not a
lookup miss its DoesNotExistException catch could absorb.
Null ids are now skipped, returning null in their place rather than being
filtered out: Flow\SynchronizationRunNode::objectsFrom() pairs
`_embed.contracts` with `contracts` by position, so dropping from one alone
would mis-attribute every later object in a flow fan-out.
That left a 100-object dry run answering with three hundred nulls across
`contracts`, `logs` and `_embed.contracts`, saying nothing the object
tallies do not already say. SynchronizationLogService had a normaliser for
exactly this but applied it only to the copy headed for storage, so the API
response and the persisted row disagreed — the response carried the nulls,
the row did not. It now normalises the log itself, which is what both are
serialised from, compacting `_embed.contracts` in lockstep with `contracts`
and covering `logs` too.
The existing REQ-011 tests all stub synchronizeContract with a uuid-bearing
contract, which is why none of them caught this. The new test uses what a
real first-time dry run produces, and fails with the reported message when
the guard is reverted.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 16, 2026
…ection The 6 PHPUnit cells failed with `Tests: 2357, Errors: 3, Failures: 1` — the suite ran to completion, so this was never the class-load fatal that identical red cells usually mean. Four distinct causes, all read from the job log rather than inferred: 1+2. `TypeError: method_exists(): Argument #1 must be of type object|string, null given` at EndpointService.php:2063, from both checkPutMandatoryFields tests. ADR-083 rule 1 moved SchemaMapper into the constructor, so the subject no longer calls `$this->containerInterface->get('OCA\OpenRegister\Db\SchemaMapper')`. Both tests still configured a ContainerInterface double, which the code never consults — the subject held the setUp() mapper, unconfigured, whose find() returns null. Promoted that mapper to $this->schemaMapper and configured it directly. The container stubs are removed because they wired a lookup that no longer happens, not to make anything compile. 3. `ArgumentCountError: Too few arguments … 21 passed and at least 23 expected` at EndpointServiceTest.php:621. One construction site was missed, and the reason is worth recording: it is a `getMockBuilder()->setConstructorArgs([...])` ARRAY, not a `new EndpointService(` call, so a sweep over the constructor call site cannot see it. Verified mechanically afterwards — the constructor takes 24 parameters, 1 optional, and the array now holds exactly 23 entries. All other sites (ExecutionTraceIntegrationTest, EndpointServiceConsumerScopeTest, EndpointServiceTierPolicyTest, and the three in this file) already carried both arguments. 4. `RegisterDescriptorTest::testRegisterDeclaresAllSchemaSlugs` failed because attaching `sync_item_dead_letter` to register.openconnector.schemas[] is a real fix, not a mistake: #170 declared that schema in components.schemas and never registered it, so SyncItemDeadLetterService and SyncDeadLetterController — which both address it by slug — were writing and reading through a schema that answered "Schema not found". SCHEMA_SLUGS is the hand-maintained record of declared slugs and simply had no entry for it. Also closes the blind spot that let #170 ship that way. Both existing guards walk SCHEMA_SLUGS outward, so a schema present in components.schemas and absent from SCHEMA_SLUGS is invisible to one and — while it is also missing from the register list — invisible to the other. testNoSchemaIsDeclaredOutsideTheSlugList asserts the other direction. Measured before adding it: components.schemas, register.openconnector.schemas[] and SCHEMA_SLUGS are all 39 and set-equal. Tests only; phpcs and psalm are scoped to lib/, so the phpcs/phpstan/psalm/E2E work already green on this PR is untouched.
This was referenced Aug 22, 2026
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.