Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
517f650
refactor(deps): inject OpenRegister instead of looking it up (ADR-083)
rubenvdlinde Aug 14, 2026
58c3c30
refactor(deps): type-hint OpenRegister's published contract (ADR-084)
rubenvdlinde Aug 15, 2026
826bf1b
fix(adr-084): complete the conversion the first transformer under-did
rubenvdlinde Aug 15, 2026
2020f87
test: pass the ObjectServiceInterface the constructors now require
rubenvdlinde Aug 15, 2026
153e850
fix: the contract lives in Contract\, not Service\
rubenvdlinde Aug 15, 2026
5e7ae8d
ci: adopt development's Code Quality workflow — the branch had the pr…
rubenvdlinde Aug 15, 2026
3e16612
test: complete the ADR-083 constructor changes and make the stubs sat…
rubenvdlinde Aug 15, 2026
4619549
refactor: persist through the published contract, not OpenRegister's …
rubenvdlinde Aug 15, 2026
701ad8b
refactor: convert the last two MagicMapper lookups on their own terms
rubenvdlinde Aug 15, 2026
684384e
fix: finish the MagicMapper removal and wire the composition root
rubenvdlinde Aug 15, 2026
cee771e
fix: repair references to the local $objectService ADR-083 deleted
rubenvdlinde Aug 15, 2026
4f6da66
fix(tests): drop constructor arguments belonging to a same-named class
rubenvdlinde Aug 15, 2026
bc36d9c
fix: clear the docblocks and dead code ADR-083 left behind
rubenvdlinde Aug 15, 2026
67e7eff
Merge development, and restore two parameters my dangling-reference p…
rubenvdlinde Aug 15, 2026
9ecc2fa
Put register and schema back inside findAll()'s config (2 sites)
rubenvdlinde Aug 16, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 23 additions & 1 deletion lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
namespace OCA\SoftwareCatalog\AppInfo;

use OCA\Decidesk\Event\DecisionConcludedEvent;
use OCA\OpenRegister\Contract\ObjectServiceInterface;
use OCA\OpenRegister\Event\ObjectCreatedEvent;
use OCA\OpenRegister\Event\ObjectUpdatedEvent;
use OCA\OpenRegister\Event\UserProfileUpdatedEvent;
Expand Down Expand Up @@ -127,6 +128,22 @@
* @spec openspec/specs/settings-service/spec.md
*/
public function register(IRegistrationContext $context): void {

// ADR-084: services type-hint OpenRegister's PUBLISHED interface, never its
// concrete class, so this app's unit tests can mock a type they are able to
// load. Nextcloud autowires concrete classes across apps but not interfaces,
// so the binding has to be stated — and the composition root is where this
// app says how it is wired.
//
// An ALIAS, not a factory: it resolves when something actually asks for the
// interface, so an instance without OpenRegister fails at the route that
// needed the data rather than at registration. Both names are strings and
// neither triggers an autoload, which is what keeps ADR-083 rule 3's promise
// that the start screen still boots.
$context->registerServiceAlias(
ObjectServiceInterface::class,
'OCA\OpenRegister\Service\ObjectService'
);
include_once __DIR__ . '/../../vendor/autoload.php';

$this->registerHandlerServices(context: $context);
Expand Down Expand Up @@ -333,6 +350,10 @@
db: $container->get(IDBConnection::class),
contactpersonHandler: $container->get(ContactPersonHandler::class),
container: $container,
// ADR-084: the published contract, resolved through the alias
// registered above, not the concrete OpenRegister class.
objectService: $container->get(ObjectServiceInterface::class),
organisationMapper: $container->get('OCA\OpenRegister\Db\OrganisationMapper'),
);
}
);
Expand All @@ -356,7 +377,8 @@
return new GebruikSyncService(
logger: $container->get('Psr\Log\LoggerInterface'),
settingsService: $container->get(SettingsService::class),
container: $container
container: $container,
objectService: $container->get(ObjectServiceInterface::class),
);
}
);
Expand Down Expand Up @@ -705,7 +727,7 @@
$context->registerService(
ContactpersonenController::class,
function ($container) {
return new ContactpersonenController(

Check failure on line 730 in lib/AppInfo/Application.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (psalm)

TooFewArguments

lib/AppInfo/Application.php:730:12: TooFewArguments: Too few arguments for OCA\SoftwareCatalog\Controller\ContactpersonenController::__construct - expecting organisationService to be passed (see https://psalm.dev/025)

Check failure on line 730 in lib/AppInfo/Application.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (psalm)

TooFewArguments

lib/AppInfo/Application.php:730:12: TooFewArguments: Too few arguments for OCA\SoftwareCatalog\Controller\ContactpersonenController::__construct - expecting objectService to be passed (see https://psalm.dev/025)

Check failure on line 730 in lib/AppInfo/Application.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (phpstan)

Missing parameter $organisationService (OCA\OpenRegister\Service\OrganisationService) in call to OCA\SoftwareCatalog\Controller\ContactpersonenController constructor.

Check failure on line 730 in lib/AppInfo/Application.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (phpstan)

Missing parameter $objectService (OCA\OpenRegister\Contract\ObjectServiceInterface) in call to OCA\SoftwareCatalog\Controller\ContactpersonenController constructor.

Check failure on line 730 in lib/AppInfo/Application.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (psalm)

TooFewArguments

lib/AppInfo/Application.php:730:12: TooFewArguments: Too few arguments for OCA\SoftwareCatalog\Controller\ContactpersonenController::__construct - expecting organisationService to be passed (see https://psalm.dev/025)

Check failure on line 730 in lib/AppInfo/Application.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (psalm)

TooFewArguments

lib/AppInfo/Application.php:730:12: TooFewArguments: Too few arguments for OCA\SoftwareCatalog\Controller\ContactpersonenController::__construct - expecting objectService to be passed (see https://psalm.dev/025)

Check failure on line 730 in lib/AppInfo/Application.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (phpstan)

Missing parameter $organisationService (OCA\OpenRegister\Service\OrganisationService) in call to OCA\SoftwareCatalog\Controller\ContactpersonenController constructor.

Check failure on line 730 in lib/AppInfo/Application.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (phpstan)

Missing parameter $objectService (OCA\OpenRegister\Contract\ObjectServiceInterface) in call to OCA\SoftwareCatalog\Controller\ContactpersonenController constructor.
appName: self::APP_ID,
request: $container->get('OCP\IRequest'),
settingsService: $container->get(SettingsService::class),
Expand Down
57 changes: 34 additions & 23 deletions lib/Controller/ContactpersonenController.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
use OCP\Security\ISecureRandom;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
use OCA\OpenRegister\Contract\ObjectServiceInterface;
use OCA\OpenRegister\Service\OrganisationService;

/**
* Controller for managing contactpersonen and their user accounts.
Expand Down Expand Up @@ -114,7 +116,7 @@
*
* @var ContainerInterface
*/
private ContainerInterface $container;

Check failure on line 119 in lib/Controller/ContactpersonenController.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (phpstan)

Property OCA\SoftwareCatalog\Controller\ContactpersonenController::$container is never read, only written.

Check failure on line 119 in lib/Controller/ContactpersonenController.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (phpstan)

Property OCA\SoftwareCatalog\Controller\ContactpersonenController::$container is never read, only written.

/**
* Contactpersoon service for business logic.
Expand Down Expand Up @@ -152,6 +154,8 @@
ContainerInterface $container,
ISecureRandom $secureRandom,
LoggerInterface $logger,
private readonly ObjectServiceInterface $objectService,
private readonly OrganisationService $organisationService,
) {
parent::__construct(appName: $appName, request: $request);
$this->settingsService = $settingsService;
Expand Down Expand Up @@ -201,7 +205,6 @@

try {
// Get object service.
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');

// Search for contactpersonen belonging to this organisation.
// Use a more generic search that doesn't require specific register/schema.
Expand All @@ -212,7 +215,7 @@
// Let ObjectService resolve the schema.
];

$contactpersonen = $objectService->searchObjectsPaginated($searchParams);
$contactpersonen = $this->objectService->searchObjectsPaginated($searchParams);

// Enhance with user information.
//
Expand Down Expand Up @@ -308,8 +311,7 @@

$callerOrgUuid = null;
try {
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');
$callerOrgUuid = $this->resolveContactOrganisation(objectService: $objectService, username: $currentUser->getUID());
$callerOrgUuid = $this->resolveContactOrganisation(objectService: $this->objectService, username: $currentUser->getUID());
} catch (\Exception $e) {
$this->logger->warning(
'ContactpersonenController: could not resolve the caller organisation, denying contact read',
Expand Down Expand Up @@ -404,10 +406,9 @@

try {
// Get object service.
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');

// Find the contactpersoon object — bind to current tenant.
$contactPersonObject = $objectService->find(
$contactPersonObject = $this->objectService->find(
id: $contactPersonId,
register: 'voorzieningen',
schema: 'contactPerson',
Expand Down Expand Up @@ -511,7 +512,7 @@

$contactData = $this->normaliseContactDataForPersist(contactData: $contactData);

$contactPersonObject->setObject($contactData);

Check failure on line 515 in lib/Controller/ContactpersonenController.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (phpstan)

Call to an undefined method OCA\OpenRegister\Contract\ObjectEntityInterface::setObject().

Check failure on line 515 in lib/Controller/ContactpersonenController.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (phpstan)

Call to an undefined method OCA\OpenRegister\Contract\ObjectEntityInterface::setObject().

// Debug logging to understand data types before save.
$lastNameValue = $contactData['achternaam'] ?? 'not set';
Expand All @@ -531,10 +532,24 @@
]
);

// Save using MagicMapper directly to bypass schema validation.
// This avoids "Unresolved reference" errors when schema references can't be resolved.
$objectMapper = $this->container->get('OCA\OpenRegister\Db\MagicMapper');
$objectMapper->update($contactPersonObject);
// Save WITHOUT schema validation, to avoid "Unresolved reference"
// errors when schema references cannot be resolved.
//
// This used to reach into OpenRegister's MagicMapper directly — its Db
// layer — which ADR-022 exists to prevent and which no leaf app can
// load in its own tests. The published contract already exposes the
// same capability as a flag, and ObjectService::saveObject() routes
// through that very mapper with register+schema, so the magic table is
// written exactly as before.
$this->objectService->saveObject(
object: $contactData,
register: $registerId,
schema: $schemaId,
uuid: $contactPersonObject->getUuid(),
silent: true,
silent: true,

Check failure on line 550 in lib/Controller/ContactpersonenController.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (psalm)

InvalidNamedArgument

lib/Controller/ContactpersonenController.php:550:5: InvalidNamedArgument: Parameter $silent has already been used in OCA\OpenRegister\Contract\ObjectServiceInterface::saveObject (see https://psalm.dev/238)

Check failure on line 550 in lib/Controller/ContactpersonenController.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (phpstan)

Argument for parameter $silent has already been passed.

Check failure on line 550 in lib/Controller/ContactpersonenController.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (psalm)

InvalidNamedArgument

lib/Controller/ContactpersonenController.php:550:5: InvalidNamedArgument: Parameter $silent has already been used in OCA\OpenRegister\Contract\ObjectServiceInterface::saveObject (see https://psalm.dev/238)

Check failure on line 550 in lib/Controller/ContactpersonenController.php

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (phpstan)

Argument for parameter $silent has already been passed.
_validation: false
);

$this->logger->info(
'ContactpersonenController: Updated contactpersoon with username',
Expand Down Expand Up @@ -942,10 +957,8 @@
*/
private function verifyCrossTenantScope(\OCP\IUser $currentUser, string $username): ?JSONResponse {
try {
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');

$targetOrgUuid = $this->resolveContactOrganisation(objectService: $objectService, username: $username);
$callerOrgUuid = $this->resolveContactOrganisation(objectService: $objectService, username: $currentUser->getUID());
$targetOrgUuid = $this->resolveContactOrganisation(objectService: $this->objectService, username: $username);
$callerOrgUuid = $this->resolveContactOrganisation(objectService: $this->objectService, username: $currentUser->getUID());

if ($targetOrgUuid !== null && $callerOrgUuid !== null && $targetOrgUuid !== $callerOrgUuid) {
$this->logger->warning(
Expand Down Expand Up @@ -988,6 +1001,9 @@
* @spec openspec/changes/method-decomposition/tasks.md#task-5
*/
private function resolveContactOrganisation(object $objectService, string $username): ?string {
// development's side on both counts: this method TAKES $objectService as a
// parameter (my dangling-reference pass wrongly made it a property read),
// and the schema slug was renamed contactpersoon -> contactPerson there.
$results = $objectService->searchObjectsPaginated(
['username' => $username, '_limit' => 1, '_schema' => 'contactPerson']
);
Expand Down Expand Up @@ -1203,8 +1219,7 @@
}

try {
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');
$contactObject = $objectService->find(
$contactObject = $this->objectService->find(
id: $contactPersonId,
register: 'voorzieningen',
schema: 'contactPerson'
Expand Down Expand Up @@ -1597,10 +1612,8 @@

// Get organisation data from OpenRegister.
try {
$organisationService = $this->container->get('OCA\OpenRegister\Service\OrganisationService');

// Get active organisation.
$activeOrg = $organisationService->getActiveOrganisation();
$activeOrg = $this->organisationService->getActiveOrganisation();
if ($activeOrg !== null) {
$response['organisations']['active'] = [
'uuid' => $activeOrg->getUuid(),
Expand All @@ -1611,7 +1624,7 @@
}

// Get all user organisations.
$userOrgs = $organisationService->getUserOrganisations();
$userOrgs = $this->organisationService->getUserOrganisations();
foreach ($userOrgs as $org) {
$response['organisations']['all'][] = [
'uuid' => $org->getUuid(),
Expand Down Expand Up @@ -1728,15 +1741,13 @@
string $userEmail,
): void {
try {
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');

$searchParams = [
'username' => $userId,
'_limit' => 1,
'_schema' => 'contactPerson',
];

$contactpersonen = $objectService->searchObjectsPaginated($searchParams);
$contactpersonen = $this->objectService->searchObjectsPaginated($searchParams);

if (empty($contactpersonen['results']) === false) {
$contactPerson = $contactpersonen['results'][0];
Expand Down
7 changes: 3 additions & 4 deletions lib/Controller/OrganisationMembersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@
use OCP\IRequest;
use OCP\IUserManager;
use OCP\IUserSession;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
use OCA\OpenRegister\Service\OrganisationService;

/**
* Beheerder-gated grant/revoke of organisation membership for an existing
Expand All @@ -77,7 +77,6 @@ class OrganisationMembersController extends Controller {
* @param IUserSession $userSession The user session (auth guard).
* @param IGroupManager $groupManager Group membership (beheerder guard).
* @param IUserManager $userManager User lookup (existing-user-only guard).
* @param ContainerInterface $container DI container, used to reach OpenRegister's
* `OrganisationService` without a hard compile-time
* dependency on another app's class.
* @param LoggerInterface $logger Logger.
Expand All @@ -87,8 +86,8 @@ public function __construct(
private readonly IUserSession $userSession,
private readonly IGroupManager $groupManager,
private readonly IUserManager $userManager,
private readonly ContainerInterface $container,
private readonly LoggerInterface $logger,
private readonly OrganisationService $organisationService,
) {
parent::__construct(appName: Application::APP_ID, request: $request);
}//end __construct()
Expand Down Expand Up @@ -266,6 +265,6 @@ private function authorizeMaintainer(string $organisationUuid): ?JSONResponse {
* @throws \Throwable When OpenRegister is unavailable.
*/
private function getOrganisationService(): \OCA\OpenRegister\Service\OrganisationService {
return $this->container->get('OCA\OpenRegister\Service\OrganisationService');
return $this->organisationService;
}//end getOrganisationService()
}//end class
8 changes: 4 additions & 4 deletions lib/Controller/SettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
namespace OCA\SoftwareCatalog\Controller;

use OCA\OpenRegister\Service\ConfigurationService;
use OCA\OpenRegister\Service\ObjectService;
use OCA\OpenRegister\Contract\ObjectServiceInterface;
use OCA\SoftwareCatalog\Service\ArchiMateService;
use OCA\SoftwareCatalog\Service\EolSyncService;
use OCA\SoftwareCatalog\Service\OrganizationSyncService;
Expand Down Expand Up @@ -64,7 +64,7 @@ class SettingsController extends Controller {
/**
* The OpenRegister object service.
*
* @var ObjectService|null The OpenRegister object service.
* @var ObjectServiceInterface|null The OpenRegister object service.
*/
private $objectService;

Expand Down Expand Up @@ -109,11 +109,11 @@ public function __construct(
/**
* Attempts to retrieve the OpenRegister service from the container.
*
* @return ObjectService|null The OpenRegister service if available, null otherwise.
* @return ObjectServiceInterface|null The OpenRegister service if available, null otherwise.
* @throws RuntimeException If the service is not available.
* @spec openspec/specs/settings-admin-controller/spec.md
*/
public function getObjectService(): ?ObjectService {
public function getObjectService(): ?ObjectServiceInterface {
if (in_array(needle: 'openregister', haystack: $this->appManager->getInstalledApps()) === true) {
$this->objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');
return $this->objectService;
Expand Down
Loading
Loading