Skip to content

Commit 04376d4

Browse files
romanetarromanetar
committed
fix: review feedback
Signed-off-by: romanetar <roman_ag@hotmail.com>
1 parent 4cda54a commit 04376d4

5 files changed

Lines changed: 204 additions & 24 deletions

File tree

‎app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitBadgeScanApiController.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,6 @@ protected function addChild(Summit $summit, array $payload): IEntity
126126
if (is_null($current_member))
127127
throw new HTTP403ForbiddenException();
128128

129-
if (!$current_member->hasSponsorMembershipsFor($summit)) {
130-
throw new HTTP403ForbiddenException('Only sponsor users can add badge scans.');
131-
}
132-
133129
return $this->service->addBadgeScan($summit, $current_member, $payload);
134130
}
135131

‎app/Models/Foundation/Main/Member.php‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1994,14 +1994,36 @@ public function addSummitRegistrationOrder(SummitOrder $summit_order)
19941994
/**
19951995
* @param Summit $summit
19961996
* @return ArrayCollection
1997+
* @throws Exception
19971998
*/
1998-
public function getSponsorsBySummit(Summit $summit): ArrayCollection
1999+
public function getAllowedSponsorsBySummit(Summit $summit): ArrayCollection
19992000
{
2000-
return new ArrayCollection(
2001-
$this->sponsor_memberships->filter(function ($entity) use ($summit) {
2002-
return $entity->getSummitId() == $summit->getId();
2003-
})->toArray()
2004-
);
2001+
$sql = <<<SQL
2002+
SELECT su.SponsorID
2003+
FROM Sponsor_Users su
2004+
INNER JOIN Sponsor s ON s.ID = su.SponsorID
2005+
WHERE su.MemberID = :member_id
2006+
AND s.SummitID = :summit_id
2007+
AND (
2008+
JSON_CONTAINS(COALESCE(su.Permissions, '[]'), JSON_QUOTE(:slug_sponsors))
2009+
OR JSON_CONTAINS(COALESCE(su.Permissions, '[]'), JSON_QUOTE(:slug_external))
2010+
)
2011+
SQL;
2012+
$ids = $this->prepareRawSQL($sql, [
2013+
'member_id' => $this->getId(),
2014+
'summit_id' => $summit->getId(),
2015+
'slug_sponsors' => IGroup::Sponsors,
2016+
'slug_external' => IGroup::SponsorExternalUsers,
2017+
])->executeQuery()->fetchFirstColumn();
2018+
2019+
if (empty($ids)) {
2020+
return new ArrayCollection();
2021+
}
2022+
2023+
$position = array_flip($ids);
2024+
$sponsors = $this->getEM()->getRepository(Sponsor::class)->findBy(['id' => $ids]);
2025+
usort($sponsors, fn($a, $b) => $position[$a->getId()] <=> $position[$b->getId()]);
2026+
return new ArrayCollection($sponsors);
20052027
}
20062028

20072029
/**

‎app/Services/Model/Imp/SponsorUserInfoGrantService.php‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -178,24 +178,22 @@ public function addBadgeScan(Summit $summit, Member $current_member, array $data
178178
if(is_null($badge))
179179
throw new EntityNotFoundException("badge not found.");
180180

181-
$member_sponsors = $current_member->getSponsorsBySummit($summit);
181+
$member_sponsors = $current_member->getAllowedSponsorsBySummit($summit);
182182

183-
if($member_sponsors->isEmpty())
184-
throw new ValidationException("Current member does not belong to any sponsor of this summit.");
183+
if ($member_sponsors->isEmpty())
184+
throw new ValidationException("Current member does not have badge scan permissions for any sponsor of this summit.");
185185

186-
if($member_sponsors->count() === 1) {
186+
if ($member_sponsors->count() === 1) {
187187
$sponsor = $member_sponsors->first();
188188
} else {
189-
if(empty($data['sponsor_id']))
189+
if (empty($data['sponsor_id']))
190190
throw new ValidationException("sponsor_id is required when the member belongs to multiple sponsors.");
191-
$sponsor = $current_member->getSponsorBySummitAndId($summit, intval($data['sponsor_id']));
192-
if(is_null($sponsor))
191+
$sponsor_id = intval($data['sponsor_id']);
192+
$sponsor = $member_sponsors->filter(fn($s) => $s->getId() === $sponsor_id)->first();
193+
if ($sponsor === false)
193194
throw new ValidationException("Current member does not belong to the selected summit sponsor.");
194195
}
195196

196-
if(!$current_member->hasSponsorMembershipsFor($summit, $sponsor))
197-
throw new ValidationException("Current member does not have badge scan permissions for the selected sponsor.");
198-
199197
$scan = new SponsorBadgeScan();
200198
$scan->setScanDate($scan_date);
201199
$scan->setQRCode($qr_code);

‎tests/Unit/Entities/SponsorPermissionTrackingTest.php‎

Lines changed: 159 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,165 @@ public function testGetActiveSummitsSponsorMembershipsIncludesWithPermission():
233233
}
234234

235235
// -------------------------------------------------------------------------
236-
// 6. Sponsor::addUser — multi-sponsor membership
236+
// 6. Member::addSponsorPermission — concurrency
237+
// -------------------------------------------------------------------------
238+
239+
/**
240+
* Concurrent calls to addSponsorPermission for the same (member, sponsor, slug)
241+
* must not introduce duplicate entries in the Permissions JSON array.
242+
* The SELECT … FOR UPDATE row lock serialises the writers so that the
243+
* second caller reads the committed value and IF(JSON_CONTAINS(…)) is a no-op.
244+
*/
245+
public function testConcurrentAddSponsorPermissionProducesNoDuplicates(): void
246+
{
247+
if (!function_exists('pcntl_fork')) {
248+
$this->markTestSkipped('pcntl_fork() is not available in this environment');
249+
}
250+
251+
$sponsor_id = self::$sponsors[0]->getId();
252+
$member_id = self::$member->getId();
253+
$concurrency = 5;
254+
255+
// Flush and disconnect the parent before forking so children each
256+
// get a clean connection — inherited sockets are not fork-safe.
257+
self::$em->flush();
258+
self::$em->clear();
259+
self::$em->getConnection()->close();
260+
261+
$pids = [];
262+
for ($i = 0; $i < $concurrency; $i++) {
263+
$pid = pcntl_fork();
264+
if ($pid === -1) {
265+
$this->fail('pcntl_fork() failed');
266+
}
267+
if ($pid === 0) {
268+
// Child: DBAL auto-reconnects on the first query after close().
269+
try {
270+
$conn = self::$em->getConnection();
271+
$conn->beginTransaction();
272+
$member = self::$member_repository->find($member_id);
273+
$member->addSponsorPermission($sponsor_id, IGroup::Sponsors);
274+
$conn->commit();
275+
exit(0);
276+
} catch (\Throwable $e) {
277+
exit(1);
278+
}
279+
}
280+
$pids[] = $pid;
281+
}
282+
283+
// Parent: wait for all children and collect exit codes.
284+
$failed = 0;
285+
foreach ($pids as $pid) {
286+
pcntl_waitpid($pid, $status);
287+
if (pcntl_wexitstatus($status) !== 0) {
288+
$failed++;
289+
}
290+
}
291+
292+
// Reconnect the parent for the assertion query.
293+
self::$em->getConnection()->close();
294+
295+
$raw = self::$em->getConnection()->executeQuery(
296+
'SELECT Permissions FROM Sponsor_Users WHERE SponsorID = ? AND MemberID = ?',
297+
[$sponsor_id, $member_id]
298+
)->fetchOne();
299+
300+
$permissions = json_decode($raw, true) ?? [];
301+
$occurrences = array_filter($permissions, fn($p) => $p === IGroup::Sponsors);
302+
303+
$this->assertSame(0, $failed, 'One or more concurrent workers exited with an error.');
304+
$this->assertCount(
305+
1,
306+
$occurrences,
307+
'Concurrent addSponsorPermission calls must not produce duplicate slugs in Permissions.'
308+
);
309+
}
310+
311+
// -------------------------------------------------------------------------
312+
// 7. Member::removeSponsorPermission — concurrency
313+
// -------------------------------------------------------------------------
314+
315+
/**
316+
* Concurrent calls to removeSponsorPermission for the same (member, sponsor, slug)
317+
* must leave the slug completely absent from the Permissions JSON array.
318+
* The pre-loaded Permissions intentionally contains duplicate slugs to verify
319+
* that the JSON_ARRAYAGG-based remove eliminates all occurrences in one shot
320+
* and that concurrent workers do not leave stale entries behind.
321+
*/
322+
public function testConcurrentRemoveSponsorPermissionLeavesNoStaleEntries(): void
323+
{
324+
if (!function_exists('pcntl_fork')) {
325+
$this->markTestSkipped('pcntl_fork() is not available in this environment');
326+
}
327+
328+
$sponsor_id = self::$sponsors[0]->getId();
329+
$member_id = self::$member->getId();
330+
$concurrency = 5;
331+
332+
// Seed Permissions with duplicate slugs to exercise the remove-all path.
333+
self::$em->getConnection()->executeStatement(
334+
'UPDATE Sponsor_Users SET Permissions = ? WHERE SponsorID = ? AND MemberID = ?',
335+
[
336+
json_encode([IGroup::Sponsors, IGroup::Sponsors, IGroup::Sponsors]),
337+
$sponsor_id,
338+
$member_id,
339+
]
340+
);
341+
342+
self::$em->flush();
343+
self::$em->clear();
344+
self::$em->getConnection()->close();
345+
346+
$pids = [];
347+
for ($i = 0; $i < $concurrency; $i++) {
348+
$pid = pcntl_fork();
349+
if ($pid === -1) {
350+
$this->fail('pcntl_fork() failed');
351+
}
352+
if ($pid === 0) {
353+
try {
354+
$conn = self::$em->getConnection();
355+
$conn->beginTransaction();
356+
$member = self::$member_repository->find($member_id);
357+
$member->removeSponsorPermission($sponsor_id, IGroup::Sponsors);
358+
$conn->commit();
359+
exit(0);
360+
} catch (\Throwable $e) {
361+
exit(1);
362+
}
363+
}
364+
$pids[] = $pid;
365+
}
366+
367+
$failed = 0;
368+
foreach ($pids as $pid) {
369+
pcntl_waitpid($pid, $status);
370+
if (pcntl_wexitstatus($status) !== 0) {
371+
$failed++;
372+
}
373+
}
374+
375+
self::$em->getConnection()->close();
376+
377+
$raw = self::$em->getConnection()->executeQuery(
378+
'SELECT Permissions FROM Sponsor_Users WHERE SponsorID = ? AND MemberID = ?',
379+
[$sponsor_id, $member_id]
380+
)->fetchOne();
381+
382+
$permissions = json_decode($raw, true) ?? [];
383+
$occurrences = array_filter($permissions, fn($p) => $p === IGroup::Sponsors);
384+
385+
$this->assertSame(0, $failed, 'One or more concurrent workers exited with an error.');
386+
$this->assertCount(
387+
0,
388+
$occurrences,
389+
'Concurrent removeSponsorPermission calls must leave no stale slug occurrences in Permissions.'
390+
);
391+
}
392+
393+
// -------------------------------------------------------------------------
394+
// 8. Sponsor::addUser — multi-sponsor membership
237395
// -------------------------------------------------------------------------
238396

239397
/**

‎tests/oauth2/OAuth2SummitBadgeScanApiControllerTest.php‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public function testAddEncryptedBadgeScan(){
9191
self::$em->flush();
9292

9393
$this->assertTrue($sponsor->hasUser(self::$member));
94-
$this->assertGreaterThan(0, self::$member->getSponsorsBySummit(self::$summit)->count());
94+
$this->assertGreaterThan(0, self::$member->getAllowedSponsorsBySummit(self::$summit)->count());
9595

9696
$badge = $attendee->getFirstTicket()->getBadge();
9797
$badge_qr_code = $badge->generateQRCode();
@@ -371,7 +371,10 @@ public function testAddBadgeScanWithMultipleSponsorsWithoutSponsorId()
371371

372372
self::$em->flush();
373373

374-
$this->assertGreaterThan(1, self::$member->getSponsorsBySummit(self::$summit)->count());
374+
self::$member->addSponsorPermission($sponsor1->getId(), IGroup::Sponsors);
375+
self::$member->addSponsorPermission($sponsor2->getId(), IGroup::Sponsors);
376+
377+
$this->assertGreaterThan(1, self::$member->getAllowedSponsorsBySummit(self::$summit)->count());
375378

376379
$params = [
377380
'id' => self::$summit->getId(),
@@ -416,7 +419,10 @@ public function testAddBadgeScanWithMultipleSponsorsWithSponsorId()
416419

417420
self::$em->flush();
418421

419-
$this->assertGreaterThan(1, self::$member->getSponsorsBySummit(self::$summit)->count());
422+
self::$member->addSponsorPermission($sponsor1->getId(), IGroup::Sponsors);
423+
self::$member->addSponsorPermission($sponsor2->getId(), IGroup::Sponsors);
424+
425+
$this->assertGreaterThan(1, self::$member->getAllowedSponsorsBySummit(self::$summit)->count());
420426

421427
$params = [
422428
'id' => self::$summit->getId(),

0 commit comments

Comments
 (0)