Skip to content

Commit 1a1632a

Browse files
authored
fix(gate-57): delete six orphaned write capabilities (#530)
* fix(gate-57): delete six orphaned write capabilities gate-57 orphaned-write-capability reported 6 findings over 55 lib/Service files. All six had zero callers; all six are deleted. - AangebodenGebruik\GebruikStatusHandler::updateStatus — the whole class is unreferenced. Its docblock says "AangebodenGebruikService delegates all updateStatus() logic here"; AangebodenGebruikService has no such method and never constructs the handler, so the method-decomposition extraction landed without its caller ever being updated. Deleted with the class, and with StatusTransitionValidator, whose only consumer it was. - SoftwareCatalogContactSyncService::importContact — validated a UID and returned it unchanged. The two live consumers (OrganizationContactSyncJob, Repair\MigrateContactsToNc) use isAvailable/findContactByUid/ findContactForRecord/syncToContacts, which already serve resolve-or-create. Spec REQ-SCNC-003 updated: the service no longer names importContact. - SoftwareCatalogue\ContactPersonHandler::updateUserGroupsFromRoles — already @deprecated, logged "deprecated - role assignment now based on organization type" on every call, ignored both role arguments and forwarded to updateUserGroupsFromContactData(). No caller, so its backward compatibility had no consumer. Group membership is a permission fact; a second entry point into it is a surface, not a convenience. - SoftwareCatalogueService::sendGebruikerWelcomeEmail, ::syncUserWithRevertedContact and ::updateUserFromRevertedGebruiker — each was a single logger->info() and nothing else. They named capabilities (send a welcome mail, reconcile a user after an object revert) that have never been implemented; wiring a log line in would have hidden the gap. ⚠️ SoftwareCatalogEventListenerTest asserted that the listener calls sendGebruikerWelcomeEmail, syncUserWithRevertedContact and updateUserFromRevertedGebruiker exactly once each. It does not, and there is no code path in SoftwareCatalogEventListener that ever did — the tests described a wiring that does not exist. They are corrected here rather than deleted: the revert case now asserts the true invariant (the revert path runs without reaching a capability the app does not implement). Before: 6 findings over 55 files. After: 0 findings over 53 files (two service files fewer because the dead handler pair was deleted). * style(catalogue): capitalise two inline comments Two pre-existing phpcs errors ('Inline comments must start with a capital letter') in a file this branch already touches, per the fix-what-you-touch rule. lib/Service/SoftwareCatalogueService.php is now phpcs-clean (0 errors).
1 parent ece273a commit 1a1632a

7 files changed

Lines changed: 65 additions & 434 deletions

File tree

lib/Service/AangebodenGebruik/GebruikStatusHandler.php

Lines changed: 0 additions & 152 deletions
This file was deleted.

lib/Service/AangebodenGebruik/StatusTransitionValidator.php

Lines changed: 0 additions & 108 deletions
This file was deleted.

lib/Service/SoftwareCatalogContactSyncService.php

Lines changed: 11 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -113,35 +113,18 @@ public function searchContacts(string $query): array {
113113
return $contacts;
114114
}//end searchContacts()
115115

116-
/**
117-
* Import (resolve) a Nextcloud contact UID into a catalog relationship
118-
* record's `contactsUid`. Idempotent: an already-known UID is returned
119-
* unchanged. An unknown UID raises a RuntimeException.
120-
*
121-
* @param string $uid The contact UID to import.
122-
* @param ?string $addressBookKey Optional addressbook key (kept for parity
123-
* with the canonical signature; unused here).
124-
*
125-
* @return string The resolved contacts UID.
126-
*
127-
* @throws RuntimeException When Contacts is disabled or the UID is unknown.
128-
*
129-
* @SuppressWarnings(PHPMD.UnusedFormalParameter) — $addressBookKey kept for canonical-signature parity.
130-
*
131-
* @spec openspec/specs/softwarecatalog-contacts-to-nc/spec.md
116+
/*
117+
* NO importContact() HERE.
118+
*
119+
* It validated that a UID resolved and returned that same UID unchanged —
120+
* no import, no write. It had no caller: the two live consumers,
121+
* `BackgroundJob\OrganizationContactSyncJob` and `Repair\MigrateContactsToNc`,
122+
* use `isAvailable()`, `findContactByUid()`, `findContactForRecord()` and
123+
* `syncToContacts()`, which together already serve the capability
124+
* ("resolve or create the Nextcloud Contact for a record and return its
125+
* UID"). A third entry point would have duplicated `syncToContacts()`
126+
* without its create path.
132127
*/
133-
public function importContact(string $uid, ?string $addressBookKey = null): string {
134-
if ($this->isAvailable() === false) {
135-
throw new RuntimeException('Nextcloud Contacts is not available');
136-
}
137-
138-
$contact = $this->findContactByUid(uid: $uid);
139-
if ($contact === null) {
140-
throw new RuntimeException('Contact not found in any accessible addressbook');
141-
}
142-
143-
return $uid;
144-
}//end importContact()
145128

146129
/**
147130
* Resolve (or create) the Nextcloud Contact for a catalog relationship

lib/Service/SoftwareCatalogue/ContactPersonHandler.php

Lines changed: 10 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -950,51 +950,17 @@ public function updateUserGroupsFromContactData(\OCP\IUser $user, array $contact
950950
}//end try
951951
}//end updateUserGroupsFromContactData()
952952

953-
/**
954-
* Legacy method for backward compatibility - now redirects to organization type-based logic
955-
*
956-
* @param \OCP\IUser $user The user to update
957-
* @param array $newRoles The new roles (ignored - kept for compatibility)
958-
* @param array $oldRoles The old roles (ignored - kept for compatibility)
959-
*
960-
* @return void
961-
* @deprecated Use updateUserGroupsFromContactData instead
962-
* @spec openspec/specs/sc-handlers/spec.md
953+
/*
954+
* NO updateUserGroupsFromRoles() HERE.
955+
*
956+
* It was already `@deprecated`, logged "role assignment now based on
957+
* organization type" on every call, ignored both of its `$newRoles` /
958+
* `$oldRoles` arguments, and forwarded to
959+
* `updateUserGroupsFromContactData()` — the live method. It had no caller,
960+
* so the backward compatibility it existed for had no consumer either.
961+
* Group membership is a permission fact; a second, role-shaped entry point
962+
* into it is a surface, not a convenience.
963963
*/
964-
public function updateUserGroupsFromRoles(\OCP\IUser $user, array $newRoles, array $oldRoles = []): void {
965-
$this->_logger->info(
966-
'updateUserGroupsFromRoles is deprecated - role assignment now based on organization type',
967-
[
968-
'username' => $user->getUID(),
969-
'newRoles' => $newRoles,
970-
'oldRoles' => $oldRoles,
971-
]
972-
);
973-
974-
// For backward compatibility, try to find the user's contact data and update based on organization type.
975-
try {
976-
$contactObject = $this->findContactPersonByUsername(username: $user->getUID());
977-
if (empty($contactObject) === true) {
978-
$this->_logger->warning(
979-
'Could not find contact person data for user - cannot update groups',
980-
['username' => $user->getUID()]
981-
);
982-
}
983-
984-
if (empty($contactObject) === false) {
985-
$contactData = $contactObject->getObject();
986-
$this->updateUserGroupsFromContactData(user: $user, contactData: $contactData);
987-
}
988-
} catch (\Exception $e) {
989-
$this->_logger->error(
990-
'Failed to update user groups via legacy method: ' . $e->getMessage(),
991-
[
992-
'username' => $user->getUID(),
993-
'exception' => $e,
994-
]
995-
);
996-
}//end try
997-
}//end updateUserGroupsFromRoles()
998964

999965
/**
1000966
* Finds contactpersoon object by username

0 commit comments

Comments
 (0)