shahondin1624/Mitgliederverwaltung
Public Access
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 16 Wiki Activity

refactor(MemberService): extract generic syncSubEntities() method

Browse Source 
Factor out the duplicated sync logic from syncAddresses(), syncPhones(),
and syncEmails() into a single generic syncSubEntities() method that
accepts callables for fetch/update/create/delete operations. Each
specialized sync method is now a thin ~10-line wrapper.

(Closes #204)
This commit is contained in:
shahondin1624
2026-04-30 08:12:35 +02:00
parent c4f5f8e7fb
commit bfb57b4e1e
2 changed files with 80 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.plans/open/issue-204-duplicate-sync-methods.md
+7 -19
View File
@@ -46,29 +46,17 @@ private function syncAddresses(int $memberId, array $incoming): void {
## Solution
Create a generic sync utility:
```php
private function syncSubEntities(
int $memberId,
array $incoming,
callable $findByMemberId,
callable $add,
callable $update,
callable $delete
): void {
// Generic sync logic
}
```
Or create a `SubEntitySyncService` for reusability across entities.
Extract the common sync logic into a single private generic method that accepts callables for the mapper-specific operations. Each of the three sync methods becomes a thin 10-line wrapper calling the generic method.
## Tasks
- [ ] Extract common sync logic into a generic method or service
- [ ] Extract common sync logic into `syncSubEntities()` private generic method
- [ ] Refactor `syncAddresses()`, `syncPhones()`, `syncEmails()` to use the new pattern
- [ ] Ensure identical behavior is preserved (test thoroughly)
- [ ] Document the generic sync algorithm
- [ ] Ensure identical behavior is preserved (all existing tests must pass)
- [ ] Run `make test` — all must pass
- [ ] Commit, push, create PR via tea, merge it
- [ ] Move plan file from `.plans/open/` to `.plans/done/`
- [ ] Check out main again
## Labels
lib/Service/MemberService.php
+72 -66
View File
@@ -555,6 +555,51 @@ class MemberService {
return $this->find($id);
}
/**
* Generic sub-entity sync: update existing, create new, delete removed.
*
* @param int $memberId
* @param array[] $incoming Payload arrays (each may contain an 'id' key)
* @param callable(): list<object> $fetchExisting Fetch current DB entities for this member
* @param callable(object): int $getId Extract entity ID
* @param callable(int, array): void $updateEntity Update an existing entity by ID
* @param callable(int, array): void $createEntity Create a new entity for a member
* @param callable(int): void $deleteEntity Delete an entity by ID
* @throws Exception
*/
private function syncSubEntities(
int $memberId,
array $incoming,
callable $fetchExisting,
callable $getId,
callable $updateEntity,
callable $createEntity,
callable $deleteEntity
): void {
$existing = $fetchExisting();
$existingById = [];
foreach ($existing as $entity) {
$existingById[$getId($entity)] = $entity;
}
$keptIds = [];
foreach ($incoming as $item) {
$id = isset($item['id']) ? (int)$item['id'] : 0;
if ($id > 0 && isset($existingById[$id])) {
$updateEntity($id, $item);
$keptIds[$id] = true;
} else {
$createEntity($memberId, $item);
}
}
foreach (array_keys($existingById) as $id) {
if (!isset($keptIds[$id])) {
$deleteEntity($id);
}
}
}
/**
* Sync a member's addresses against an incoming array.
* Rules:
@@ -566,28 +611,15 @@ class MemberService {
* @throws Exception
*/
private function syncAddresses(int $memberId, array $incoming): void {
$existing = $this->addressMapper->findByMemberId($memberId);
$existingById = [];
foreach ($existing as $addr) {
$existingById[$addr->getId()] = $addr;
}
$keptIds = [];
foreach ($incoming as $item) {
$id = isset($item['id']) ? (int)$item['id'] : 0;
if ($id > 0 && isset($existingById[$id])) {
$this->updateAddress($id, $item);
$keptIds[$id] = true;
} else {
$this->addAddress($memberId, $item);
}
}
foreach ($existingById as $id => $addr) {
if (!isset($keptIds[$id])) {
$this->deleteAddress($id);
}
}
$this->syncSubEntities(
$memberId,
$incoming,
fn() => $this->addressMapper->findByMemberId($memberId),
fn($e) => $e->getId(),
fn($id, $data) => $this->updateAddress($id, $data),
fn($mid, $data) => $this->addAddress($mid, $data),
fn($id) => $this->deleteAddress($id)
);
}
/**
@@ -597,28 +629,15 @@ class MemberService {
* @throws Exception
*/
private function syncPhones(int $memberId, array $incoming): void {
$existing = $this->phoneMapper->findByMemberId($memberId);
$existingById = [];
foreach ($existing as $p) {
$existingById[$p->getId()] = $p;
}
$keptIds = [];
foreach ($incoming as $item) {
$id = isset($item['id']) ? (int)$item['id'] : 0;
if ($id > 0 && isset($existingById[$id])) {
$this->updatePhone($id, $item);
$keptIds[$id] = true;
} else {
$this->addPhone($memberId, $item);
}
}
foreach ($existingById as $id => $p) {
if (!isset($keptIds[$id])) {
$this->deletePhone($id);
}
}
$this->syncSubEntities(
$memberId,
$incoming,
fn() => $this->phoneMapper->findByMemberId($memberId),
fn($e) => $e->getId(),
fn($id, $data) => $this->updatePhone($id, $data),
fn($mid, $data) => $this->addPhone($mid, $data),
fn($id) => $this->deletePhone($id)
);
}
/**
@@ -628,28 +647,15 @@ class MemberService {
* @throws Exception
*/
private function syncEmails(int $memberId, array $incoming): void {
$existing = $this->emailMapper->findByMemberId($memberId);
$existingById = [];
foreach ($existing as $e) {
$existingById[$e->getId()] = $e;
}
$keptIds = [];
foreach ($incoming as $item) {
$id = isset($item['id']) ? (int)$item['id'] : 0;
if ($id > 0 && isset($existingById[$id])) {
$this->updateEmail($id, $item);
$keptIds[$id] = true;
} else {
$this->addEmail($memberId, $item);
}
}
foreach ($existingById as $id => $e) {
if (!isset($keptIds[$id])) {
$this->deleteEmail($id);
}
}
$this->syncSubEntities(
$memberId,
$incoming,
fn() => $this->emailMapper->findByMemberId($memberId),
fn($e) => $e->getId(),
fn($id, $data) => $this->updateEmail($id, $data),
fn($mid, $data) => $this->addEmail($mid, $data),
fn($id) => $this->deleteEmail($id)
);
}
// ── Delete (soft) ────────────────────────────────────────────────