Skip to content

Commit 517f650

Browse files
rubenvdlinderubenvdlinde
committed
refactor(deps): inject OpenRegister instead of looking it up (ADR-083)
5 file(s) reached OpenRegister through $this->container->get(...) on an UNCONDITIONAL path — no availability check, no degrading catch. The dependency was announced nowhere: not in the constructor, not in the use block, not in any type. It appeared mid-method, as a string. Now constructor-injected and typed, so the dependency is visible to a reader and to tooling. Behaviour is unchanged: the same object, from the same container, resolved at construction instead of at first use. ContainerInterface is dropped only where nothing else used it. Deliberately NOT converted, because they are correct as written (ADR-083 rule 1's exception): lookups behind isInstalled()/getInstalledApps(), and lookups whose catch degrades rather than rethrows. Verified per file: php -l clean, and gate-66's lookup check reports zero remaining findings for each file changed. gate-66 for this app: 23 -> 8.
1 parent aab66d5 commit 517f650

5 files changed

Lines changed: 73 additions & 94 deletions

File tree

‎lib/Controller/ContactpersonenController.php‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@
3535
use OCP\Security\ISecureRandom;
3636
use Psr\Container\ContainerInterface;
3737
use Psr\Log\LoggerInterface;
38+
use OCA\OpenRegister\Service\ObjectService;
39+
use OCA\OpenRegister\Db\MagicMapper;
40+
use OCA\OpenRegister\Service\OrganisationService;
3841

3942
/**
4043
* Controller for managing contactpersonen and their user accounts.
@@ -152,6 +155,9 @@ public function __construct(
152155
ContainerInterface $container,
153156
ISecureRandom $secureRandom,
154157
LoggerInterface $logger,
158+
private readonly ObjectService $objectService,
159+
private readonly MagicMapper $magicMapper,
160+
private readonly OrganisationService $organisationService,
155161
) {
156162
parent::__construct(appName: $appName, request: $request);
157163
$this->settingsService = $settingsService;
@@ -201,7 +207,6 @@ public function getContactpersonen(string $organisationId): JSONResponse {
201207

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

206211
// Search for contactpersonen belonging to this organisation.
207212
// Use a more generic search that doesn't require specific register/schema.
@@ -212,7 +217,7 @@ public function getContactpersonen(string $organisationId): JSONResponse {
212217
// Let ObjectService resolve the schema.
213218
];
214219

215-
$contactpersonen = $objectService->searchObjectsPaginated($searchParams);
220+
$contactpersonen = $this->objectService->searchObjectsPaginated($searchParams);
216221

217222
// Enhance with user information.
218223
//
@@ -308,7 +313,6 @@ private function checkOrganisationReadPermission(\OCP\IUser $currentUser, string
308313

309314
$callerOrgUuid = null;
310315
try {
311-
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');
312316
$callerOrgUuid = $this->resolveContactOrganisation(objectService: $objectService, username: $currentUser->getUID());
313317
} catch (\Exception $e) {
314318
$this->logger->warning(
@@ -404,10 +408,9 @@ public function convertToUser(string $contactPersonId): JSONResponse {
404408

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

409412
// Find the contactpersoon object — bind to current tenant.
410-
$contactPersonObject = $objectService->find(
413+
$contactPersonObject = $this->objectService->find(
411414
id: $contactPersonId,
412415
register: 'voorzieningen',
413416
schema: 'contactpersoon',
@@ -533,8 +536,7 @@ public function convertToUser(string $contactPersonId): JSONResponse {
533536

534537
// Save using MagicMapper directly to bypass schema validation.
535538
// This avoids "Unresolved reference" errors when schema references can't be resolved.
536-
$objectMapper = $this->container->get('OCA\OpenRegister\Db\MagicMapper');
537-
$objectMapper->update($contactPersonObject);
539+
$this->magicMapper->update($contactPersonObject);
538540

539541
$this->logger->info(
540542
'ContactpersonenController: Updated contactpersoon with username',
@@ -942,8 +944,6 @@ private function checkGroupUpdatePermission(\OCP\IUser $currentUser, string $use
942944
*/
943945
private function verifyCrossTenantScope(\OCP\IUser $currentUser, string $username): ?JSONResponse {
944946
try {
945-
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');
946-
947947
$targetOrgUuid = $this->resolveContactOrganisation(objectService: $objectService, username: $username);
948948
$callerOrgUuid = $this->resolveContactOrganisation(objectService: $objectService, username: $currentUser->getUID());
949949

@@ -988,7 +988,7 @@ private function verifyCrossTenantScope(\OCP\IUser $currentUser, string $usernam
988988
* @spec openspec/changes/method-decomposition/tasks.md#task-5
989989
*/
990990
private function resolveContactOrganisation(object $objectService, string $username): ?string {
991-
$results = $objectService->searchObjectsPaginated(
991+
$results = $this->objectService->searchObjectsPaginated(
992992
['username' => $username, '_limit' => 1, '_schema' => 'contactpersoon']
993993
);
994994

@@ -1203,8 +1203,7 @@ public function getUserInfo(string $contactPersonId): JSONResponse {
12031203
}
12041204

12051205
try {
1206-
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');
1207-
$contactObject = $objectService->find(
1206+
$contactObject = $this->objectService->find(
12081207
id: $contactPersonId,
12091208
register: 'voorzieningen',
12101209
schema: 'contactpersoon'
@@ -1597,10 +1596,8 @@ public function getMe(): JSONResponse {
15971596

15981597
// Get organisation data from OpenRegister.
15991598
try {
1600-
$organisationService = $this->container->get('OCA\OpenRegister\Service\OrganisationService');
1601-
16021599
// Get active organisation.
1603-
$activeOrg = $organisationService->getActiveOrganisation();
1600+
$activeOrg = $this->organisationService->getActiveOrganisation();
16041601
if ($activeOrg !== null) {
16051602
$response['organisations']['active'] = [
16061603
'uuid' => $activeOrg->getUuid(),
@@ -1611,7 +1608,7 @@ public function getMe(): JSONResponse {
16111608
}
16121609

16131610
// Get all user organisations.
1614-
$userOrgs = $organisationService->getUserOrganisations();
1611+
$userOrgs = $this->organisationService->getUserOrganisations();
16151612
foreach ($userOrgs as $org) {
16161613
$response['organisations']['all'][] = [
16171614
'uuid' => $org->getUuid(),
@@ -1728,15 +1725,13 @@ private function enrichMeWithContactPersonData(
17281725
string $userEmail,
17291726
): void {
17301727
try {
1731-
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');
1732-
17331728
$searchParams = [
17341729
'username' => $userId,
17351730
'_limit' => 1,
17361731
'_schema' => 'contactpersoon',
17371732
];
17381733

1739-
$contactpersonen = $objectService->searchObjectsPaginated($searchParams);
1734+
$contactpersonen = $this->objectService->searchObjectsPaginated($searchParams);
17401735

17411736
if (empty($contactpersonen['results']) === false) {
17421737
$contactPerson = $contactpersonen['results'][0];

‎lib/Controller/OrganisationMembersController.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@
5252
use OCP\IRequest;
5353
use OCP\IUserManager;
5454
use OCP\IUserSession;
55-
use Psr\Container\ContainerInterface;
5655
use Psr\Log\LoggerInterface;
56+
use OCA\OpenRegister\Service\OrganisationService;
5757

5858
/**
5959
* Beheerder-gated grant/revoke of organisation membership for an existing
@@ -87,8 +87,8 @@ public function __construct(
8787
private readonly IUserSession $userSession,
8888
private readonly IGroupManager $groupManager,
8989
private readonly IUserManager $userManager,
90-
private readonly ContainerInterface $container,
9190
private readonly LoggerInterface $logger,
91+
private readonly OrganisationService $organisationService,
9292
) {
9393
parent::__construct(appName: Application::APP_ID, request: $request);
9494
}//end __construct()
@@ -266,6 +266,6 @@ private function authorizeMaintainer(string $organisationUuid): ?JSONResponse {
266266
* @throws \Throwable When OpenRegister is unavailable.
267267
*/
268268
private function getOrganisationService(): \OCA\OpenRegister\Service\OrganisationService {
269-
return $this->container->get('OCA\OpenRegister\Service\OrganisationService');
269+
return $this->organisationService;
270270
}//end getOrganisationService()
271271
}//end class

‎lib/EventListener/UserProfileUpdatedEventListener.php‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@
2525
use OCP\EventDispatcher\IEventListener;
2626
use Psr\Container\ContainerInterface;
2727
use Psr\Log\LoggerInterface;
28+
use OCA\OpenRegister\Service\ObjectService;
29+
use OCA\OpenRegister\Db\SchemaMapper;
30+
use OCA\OpenRegister\Db\RegisterMapper;
31+
use OCA\OpenRegister\Service\Object\SaveObject\MetadataHydrationHandler;
32+
use OCA\OpenRegister\Db\MagicMapper;
2833

2934
/**
3035
* Syncs user profile changes to the corresponding contactpersoon object.
@@ -56,6 +61,11 @@ class UserProfileUpdatedEventListener implements IEventListener {
5661
*/
5762
public function __construct(
5863
private readonly ContainerInterface $container,
64+
private readonly ObjectService $objectService,
65+
private readonly SchemaMapper $schemaMapper,
66+
private readonly RegisterMapper $registerMapper,
67+
private readonly MetadataHydrationHandler $metadataHydrationHandler,
68+
private readonly MagicMapper $magicMapper,
5969
) {
6070
}//end __construct()
6171

@@ -128,7 +138,6 @@ public function handle(Event $event): void {
128138
* @spec openspec/specs/method-decomposition/spec.md
129139
*/
130140
private function syncToContactPerson(UserProfileUpdatedEvent $event, LoggerInterface $logger): void {
131-
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');
132141
$settingsService = $this->container->get(SettingsService::class);
133142

134143
$voorzieningenConfig = $settingsService->getVoorzieningenConfig();
@@ -283,15 +292,11 @@ private function persistContactPersonPatch(
283292
int $schema,
284293
LoggerInterface $logger,
285294
): void {
286-
$schemaMapper = $this->container->get('OCA\OpenRegister\Db\SchemaMapper');
287-
$registerMapper = $this->container->get('OCA\OpenRegister\Db\RegisterMapper');
288-
$metaHydrationHandler = $this->container->get('OCA\OpenRegister\Service\Object\SaveObject\MetadataHydrationHandler');
289-
290295
$schemaEntity = null;
291296
$registerEntity = null;
292297
try {
293-
$schemaEntity = $schemaMapper->find(id: $schema, _rbac: false, _multitenancy: false);
294-
$registerEntity = $registerMapper->find(id: $register, _rbac: false, _multitenancy: false);
298+
$schemaEntity = $this->schemaMapper->find(id: $schema, _rbac: false, _multitenancy: false);
299+
$registerEntity = $this->registerMapper->find(id: $register, _rbac: false, _multitenancy: false);
295300
} catch (\Exception $e) {
296301
$logger->warning(
297302
'[UserProfileUpdatedEventListener] Could not load schema/register entities for _name hydration',
@@ -302,7 +307,7 @@ private function persistContactPersonPatch(
302307
}
303308

304309
if ($schemaEntity !== null) {
305-
$metaHydrationHandler->hydrateObjectMetadata(entity: $contactPerson, schema: $schemaEntity);
310+
$this->metadataHydrationHandler->hydrateObjectMetadata(entity: $contactPerson, schema: $schemaEntity);
306311
$logger->debug(
307312
'[UserProfileUpdatedEventListener] Regenerated _name metadata',
308313
[
@@ -313,8 +318,7 @@ private function persistContactPersonPatch(
313318

314319
// Pass register and schema so the magic mapper route is triggered and the
315320
// per-schema magic table is updated (not just the blob table).
316-
$objectMapper = $this->container->get('OCA\OpenRegister\Db\MagicMapper');
317-
$objectMapper->update(entity: $contactPerson, register: $registerEntity, schema: $schemaEntity);
321+
$this->magicMapper->update(entity: $contactPerson, register: $registerEntity, schema: $schemaEntity);
318322

319323
}//end persistContactpersoonPatch()
320324

@@ -341,7 +345,7 @@ private function findContactPerson(
341345
// 1. Search by username = userId, scoped to the user's organisation (multitenancy).
342346
// This prevents updating a contactpersoon from a different organisation when.
343347
// Multiple records share the same username across orgs.
344-
$results = $objectService->searchObjects(
348+
$results = $this->objectService->searchObjects(
345349
query: ['@self' => $selfQuery, 'username' => $userId, '_limit' => 5],
346350
_rbac: false,
347351
_multitenancy: true
@@ -381,7 +385,7 @@ private function findContactPerson(
381385

382386
// Use _search for case-insensitive matching, then verify the email field in PHP.
383387
// Scoped to user's organisation via multitenancy to avoid cross-org matches.
384-
$results = $objectService->searchObjects(
388+
$results = $this->objectService->searchObjects(
385389
query: ['@self' => $selfQuery, '_search' => $emailCandidate, '_limit' => 5],
386390
_rbac: false,
387391
_multitenancy: true

‎lib/Service/GebruikSyncService.php‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
use OCA\OpenRegister\Db\ObjectEntity;
3030
use Psr\Container\ContainerInterface;
3131
use Psr\Log\LoggerInterface;
32+
use OCA\OpenRegister\Service\ObjectService;
3233

3334
/**
3435
* Service for synchronizing and processing Gebruik (Usage) objects.
@@ -86,6 +87,7 @@ public function __construct(
8687
LoggerInterface $logger,
8788
SettingsService $settingsService,
8889
ContainerInterface $container,
90+
private readonly ObjectService $objectService,
8991
) {
9092
$this->logger = $logger;
9193
$this->settingsService = $settingsService;
@@ -324,7 +326,6 @@ private function processAmefElements(ObjectEntity $gebruikObject): array {
324326
* @return array Array of found ObjectEntity objects.
325327
*/
326328
private function searchAmefElementsByIds(array $ids, string $register, string $schema): array {
327-
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');
328329
$foundElements = [];
329330

330331
foreach ($ids as $id) {
@@ -340,7 +341,7 @@ private function searchAmefElementsByIds(array $ids, string $register, string $s
340341
'_limit' => 5,
341342
];
342343

343-
$elements = $objectService->searchObjects($query);
344+
$elements = $this->objectService->searchObjects($query);
344345
$foundElements = array_merge($foundElements, $elements);
345346
} catch (Exception $e) {
346347
$this->logger->warning(
@@ -515,8 +516,6 @@ private function resolveLatestEligibleStatus(array $statusDates, string $gebruik
515516
*/
516517
private function updateGebruikObject(ObjectEntity $gebruikObject, array $updatedData): void {
517518
try {
518-
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');
519-
520519
// Get voorzieningenConfig to find the correct register and schema.
521520
$voorzieningenConfig = $this->settingsService->getVoorzieningenConfig();
522521
$register = $voorzieningenConfig['register'] ?? '';
@@ -527,7 +526,7 @@ private function updateGebruikObject(ObjectEntity $gebruikObject, array $updated
527526
}
528527

529528
// Update the object.
530-
$objectService->saveObject(
529+
$this->objectService->saveObject(
531530
object: $updatedData,
532531
register: (int)$register,
533532
schema: (int)$gebruikSchema,

0 commit comments

Comments
 (0)