Skip to content

refactor(deps): inject OpenRegister instead of looking it up (ADR-083) #515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
rubenvdlinde wants to merge 1 commit into
base: development
Choose a base branch
from
Draft

refactor(deps): inject OpenRegister instead of looking it up (ADR-083) #515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 14 additions & 19 deletions lib/Controller/ContactpersonenController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
use OCP\Security\ISecureRandom;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
use OCA\OpenRegister\Service\ObjectService;
use OCA\OpenRegister\Db\MagicMapper;
use OCA\OpenRegister\Service\OrganisationService;

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

Check failure on line 120 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 +155,9 @@
ContainerInterface $container,
ISecureRandom $secureRandom,
LoggerInterface $logger,
private readonly ObjectService $objectService,
private readonly MagicMapper $magicMapper,

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

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (psalm)

UndefinedClass 

lib/Controller/ContactpersonenController.php:159:3: UndefinedClass: Class, interface or enum named OCA\OpenRegister\Db\MagicMapper does not exist (see https://psalm.dev/019)
private readonly OrganisationService $organisationService,
) {
parent::__construct(appName: $appName, request: $request);
$this->settingsService = $settingsService;
Expand Down Expand Up @@ -201,7 +207,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 +217,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 +313,7 @@

$callerOrgUuid = null;
try {
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');
$callerOrgUuid = $this->resolveContactOrganisation(objectService: $objectService, username: $currentUser->getUID());

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

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (phpstan) 

Undefined variable: $objectService

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

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (psalm)

UndefinedVariable 

lib/Controller/ContactpersonenController.php:316:70: UndefinedVariable: Cannot find referenced variable $objectService (see https://psalm.dev/024)
} catch (\Exception $e) {
$this->logger->warning(
'ContactpersonenController: could not resolve the caller organisation, denying contact read',
Expand Down Expand Up @@ -404,10 +408,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: 'contactpersoon',
Expand Down Expand Up @@ -533,8 +536,7 @@

// 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);
$this->magicMapper->update($contactPersonObject);

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

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (psalm)

UndefinedClass 

lib/Controller/ContactpersonenController.php:539:4: UndefinedClass: Class, interface or enum named OCA\OpenRegister\Db\MagicMapper does not exist (see https://psalm.dev/019)

$this->logger->info(
'ContactpersonenController: Updated contactpersoon with username',
Expand Down Expand Up @@ -942,9 +944,7 @@
*/
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);

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

View workflow job for this annotation

GitHub Actions / quality / PHP Quality (phpstan) 

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

if ($targetOrgUuid !== null && $callerOrgUuid !== null && $targetOrgUuid !== $callerOrgUuid) {
Expand Down Expand Up @@ -988,7 +988,7 @@
* @spec openspec/changes/method-decomposition/tasks.md#task-5
*/
private function resolveContactOrganisation(object $objectService, string $username): ?string {
$results = $objectService->searchObjectsPaginated(
$results = $this->objectService->searchObjectsPaginated(
['username' => $username, '_limit' => 1, '_schema' => 'contactpersoon']
);

Expand Down Expand Up @@ -1203,8 +1203,7 @@
}

try {
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');
$contactObject = $objectService->find(
$contactObject = $this->objectService->find(
id: $contactPersonId,
register: 'voorzieningen',
schema: 'contactpersoon'
Expand Down Expand Up @@ -1597,10 +1596,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 +1608,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 +1725,13 @@
string $userEmail,
): void {
try {
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');

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

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

if (empty($contactpersonen['results']) === false) {
$contactPerson = $contactpersonen['results'][0];
Expand Down
6 changes: 3 additions & 3 deletions lib/Controller/OrganisationMembersController.php
View file
Open in desktop
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 Down Expand Up @@ -87,8 +87,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 +266,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
28 changes: 16 additions & 12 deletions lib/EventListener/UserProfileUpdatedEventListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@
use OCP\EventDispatcher\IEventListener;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
use OCA\OpenRegister\Service\ObjectService;
use OCA\OpenRegister\Db\SchemaMapper;
use OCA\OpenRegister\Db\RegisterMapper;
use OCA\OpenRegister\Service\Object\SaveObject\MetadataHydrationHandler;
use OCA\OpenRegister\Db\MagicMapper;

/**
* Syncs user profile changes to the corresponding contactpersoon object.
Expand Down Expand Up @@ -56,6 +61,11 @@ class UserProfileUpdatedEventListener implements IEventListener {
*/
public function __construct(
private readonly ContainerInterface $container,
private readonly ObjectService $objectService,
private readonly SchemaMapper $schemaMapper,
private readonly RegisterMapper $registerMapper,
private readonly MetadataHydrationHandler $metadataHydrationHandler,
private readonly MagicMapper $magicMapper,
) {
}//end __construct()

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

$voorzieningenConfig = $settingsService->getVoorzieningenConfig();
Expand Down Expand Up @@ -283,15 +292,11 @@ private function persistContactPersonPatch(
int $schema,
LoggerInterface $logger,
): void {
$schemaMapper = $this->container->get('OCA\OpenRegister\Db\SchemaMapper');
$registerMapper = $this->container->get('OCA\OpenRegister\Db\RegisterMapper');
$metaHydrationHandler = $this->container->get('OCA\OpenRegister\Service\Object\SaveObject\MetadataHydrationHandler');

$schemaEntity = null;
$registerEntity = null;
try {
$schemaEntity = $schemaMapper->find(id: $schema, _rbac: false, _multitenancy: false);
$registerEntity = $registerMapper->find(id: $register, _rbac: false, _multitenancy: false);
$schemaEntity = $this->schemaMapper->find(id: $schema, _rbac: false, _multitenancy: false);
$registerEntity = $this->registerMapper->find(id: $register, _rbac: false, _multitenancy: false);
} catch (\Exception $e) {
$logger->warning(
'[UserProfileUpdatedEventListener] Could not load schema/register entities for _name hydration',
Expand All @@ -302,7 +307,7 @@ private function persistContactPersonPatch(
}

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

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

}//end persistContactpersoonPatch()

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

// Use _search for case-insensitive matching, then verify the email field in PHP.
// Scoped to user's organisation via multitenancy to avoid cross-org matches.
$results = $objectService->searchObjects(
$results = $this->objectService->searchObjects(
query: ['@self' => $selfQuery, '_search' => $emailCandidate, '_limit' => 5],
_rbac: false,
_multitenancy: true
Expand Down
9 changes: 4 additions & 5 deletions lib/Service/GebruikSyncService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use OCA\OpenRegister\Db\ObjectEntity;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
use OCA\OpenRegister\Service\ObjectService;

/**
* Service for synchronizing and processing Gebruik (Usage) objects.
Expand Down Expand Up @@ -86,6 +87,7 @@ public function __construct(
LoggerInterface $logger,
SettingsService $settingsService,
ContainerInterface $container,
private readonly ObjectService $objectService,
) {
$this->logger = $logger;
$this->settingsService = $settingsService;
Expand Down Expand Up @@ -324,7 +326,6 @@ private function processAmefElements(ObjectEntity $gebruikObject): array {
* @return array Array of found ObjectEntity objects.
*/
private function searchAmefElementsByIds(array $ids, string $register, string $schema): array {
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');
$foundElements = [];

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

$elements = $objectService->searchObjects($query);
$elements = $this->objectService->searchObjects($query);
$foundElements = array_merge($foundElements, $elements);
} catch (Exception $e) {
$this->logger->warning(
Expand Down Expand Up @@ -515,8 +516,6 @@ private function resolveLatestEligibleStatus(array $statusDates, string $gebruik
*/
private function updateGebruikObject(ObjectEntity $gebruikObject, array $updatedData): void {
try {
$objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');

// Get voorzieningenConfig to find the correct register and schema.
$voorzieningenConfig = $this->settingsService->getVoorzieningenConfig();
$register = $voorzieningenConfig['register'] ?? '';
Expand All @@ -527,7 +526,7 @@ private function updateGebruikObject(ObjectEntity $gebruikObject, array $updated
}

// Update the object.
$objectService->saveObject(
$this->objectService->saveObject(
object: $updatedData,
register: (int)$register,
schema: (int)$gebruikSchema,
Expand Down
Loading
Loading