Add debug logging of the usage of array access methods so that we can… (

#3013) * Add debug logging of the usage of array access methods so that we can discover all the hidden places it's being used. * Cludgy fix to get the static providers giving valid entity objects.
ministryofjustice · Dec 19, 2024 · 6c72e3f · 6c72e3f
1 parent ed194b6
commit 6c72e3f
Show file tree

Hide file tree

Showing 16 changed files with 431 additions and 237 deletions.
diff --git a/service-api/app/features/context/Integration/LpaContext.php b/service-api/app/features/context/Integration/LpaContext.php
@@ -22,6 +22,7 @@
 use App\Service\Lpa\FindActorInLpa\ActorMatch;
 use App\Service\Lpa\GetInstructionsAndPreferencesImages;
 use App\Service\Lpa\RemoveLpa;
+use App\Service\Lpa\SiriusLpa;
 use App\Service\Lpa\SiriusLpaManager;
 use App\Service\Lpa\SiriusPerson;
 use App\Service\ViewerCodes\ViewerCodeService;
@@ -41,6 +42,7 @@
 use PHPUnit\Framework\Assert;
 use PHPUnit\Framework\ExpectationFailedException;
 use Psr\Http\Message\RequestInterface;
+use Psr\Log\LoggerInterface;
 use stdClass;
 
 use function PHPUnit\Framework\assertEquals;
@@ -386,13 +388,22 @@ public function iProvideTheAttorneyDetailsFromAValidPaperLPADocument(): void
         $lpaMatchResponse = $addOlderLpa->validateRequest($this->userId, $data);
 
         $expectedLpaArray = json_decode(json_encode($this->lpa), true);
-        $expectedLpa      = new \App\Service\Lpa\SiriusLpa($expectedLpaArray);
+        $expectedLpa      = new SiriusLpa($expectedLpaArray, $this->container->get(LoggerInterface::class));
         $expectedResponse = new AccessForAllValidation(
             new ActorMatch(
-                new SiriusPerson(json_decode(json_encode($this->lpa->attorneys[0]), true)),
+                new SiriusPerson(
+                    json_decode(
+                        json_encode(
+                            $this->lpa->attorneys[0]
+                        ),
+                        true
+                    ),
+                    $this->container->get(LoggerInterface::class),
+                ),
                 'attorney',
-                $this->lpa->uId),
-                $expectedLpa,
+                $this->lpa->uId,
+            ),
+            $expectedLpa,
             null,
         );
 
@@ -1594,13 +1605,20 @@ public function iConfirmDetailsShownToMeOfTheFoundLPAAreCorrect(): void
         $lpaMatchResponse = $addOlderLpa->validateRequest($this->userId, $data);
 
         $expectedLpaArray = json_decode(json_encode($this->lpa), true);
-        $expectedLpa      = new \App\Service\Lpa\SiriusLpa($expectedLpaArray);
+        $expectedLpa      = new SiriusLpa($expectedLpaArray, $this->container->get(LoggerInterface::class));
         $expectedResponse = new AccessForAllValidation(
             new ActorMatch(
-                new SiriusPerson(json_decode(json_encode($lpa->donor), true)),
+                new SiriusPerson(
+                    json_decode(
+                        json_encode($lpa->donor),
+                        true
+                    ),
+                    $this->container->get(LoggerInterface::class),
+                ),
                 'donor',
-                $lpa->uId),
-                $expectedLpa,
+                $lpa->uId,
+            ),
+            $expectedLpa,
             null,
         );
 
@@ -1823,68 +1841,6 @@ public function iProvideDetailsThatDoNotMatchThePaperDocument($firstnames, $last
         throw new ExpectationFailedException('LPA should not have matched data provided');
     }
 
-    /**
-     * @When /^I provide details "([^"]*)" "([^"]*)" "([^"]*)" "([^"]*)" that match a valid paper document$/
-     */
-    public function iProvideDetailsThatMatchAValidPaperDocument($firstnames, $lastname, $postcode, $dob)
-    {
-        $this->lpa = json_decode(file_get_contents(__DIR__ . '../../../../test/fixtures/test_lpa.json'));
-
-        $this->lpa->donor->firstname = 'Rachel';
-        $this->lpa->donor->surname   =  'S’anderson';
-
-        $data = [
-            'reference_number'     => (int) $this->lpaUid,
-            'dob'                  => $dob,
-            'postcode'             => $postcode,
-            'first_names'          => $firstnames,
-            'last_name'            => $lastname,
-            'force_activation_key' => false,
-        ];
-
-        //UserLpaActorMap: getAllForUser
-        $this->awsFixtures->append(
-            new Result([])
-        );
-
-        $this->pactGetInteraction(
-            $this->apiGatewayPactProvider,
-            '/v1/use-an-lpa/lpas/' . $this->lpaUid,
-            StatusCodeInterface::STATUS_OK,
-            $this->lpa
-        );
-
-        $codeExists          = new stdClass();
-        $codeExists->Created = null;
-
-        $this->pactPostInteraction(
-            $this->codesApiPactProvider,
-            '/v1/exists',
-            [
-                'lpa'   => $this->lpa->uId,
-                'actor' => $this->lpa->donor->uId,
-            ],
-            StatusCodeInterface::STATUS_OK,
-            $codeExists
-        );
-
-        $addAccessForAllLpa = $this->container->get(AddAccessForAllLpa::class);
-        $lpaMatchResponse   = $addAccessForAllLpa->validateRequest($this->userId, $data);
-
-        $expectedLpaArray = json_decode(json_encode($this->lpa), true);
-        $expectedLpa      = new \App\Service\Lpa\SiriusLpa($expectedLpaArray);
-        $expectedResponse = new AccessForAllValidation(
-            new ActorMatch(
-                new SiriusPerson(json_decode(json_encode($this->lpa->donor), true)),
-                'donor',
-                $this->lpa->uId),
-                $expectedLpa,
-        null
-        );
-
-        Assert::assertEquals($expectedResponse, $lpaMatchResponse);
-    }
-
     /**
      * @When /^I provide the details from a valid paper LPA document$/
      */
@@ -1932,13 +1888,20 @@ public function iProvideTheDetailsFromAValidPaperLPADocument(): void
         $lpaMatchResponse = $addOlderLpa->validateRequest($this->userId, $data);
 
         $expectedLpaArray = json_decode(json_encode($this->lpa), true);
-        $expectedLpa      = new \App\Service\Lpa\SiriusLpa($expectedLpaArray);
+        $expectedLpa      = new SiriusLpa($expectedLpaArray, $this->container->get(LoggerInterface::class));
         $expectedResponse = new AccessForAllValidation(
             new ActorMatch(
-                new SiriusPerson(json_decode(json_encode($this->lpa->donor), true)),
+                new SiriusPerson(
+                    json_decode(
+                        json_encode($this->lpa->donor),
+                        true
+                    ),
+                    $this->container->get(LoggerInterface::class)
+                ),
                 'donor',
-                    $this->lpa->uId),
-                    $expectedLpa,
+                $this->lpa->uId,
+            ),
+            $expectedLpa,
             null
         );
 
@@ -1971,7 +1934,7 @@ public function iProvideTheDetailsFromAValidPaperDocumentThatAlreadyHasAnActivat
             $this->lpa
         );
 
-        $codeExists = new stdClass();
+        $codeExists  = new stdClass();
         $createdDate = (new DateTime())->modify('-14 days');
 
         $activationKeyDueDate = DateTimeImmutable::createFromMutable($createdDate);
@@ -2001,11 +1964,19 @@ public function iProvideTheDetailsFromAValidPaperDocumentThatAlreadyHasAnActivat
             Assert::assertEquals('LPA has an activation key already', $ex->getMessage());
             Assert::assertEquals(
                 [
-                    'donor'                => new SiriusPerson(json_decode(json_encode($this->lpa->donor), true)),
+                    'donor'                => new SiriusPerson(
+                        json_decode(
+                            json_encode(
+                                $this->lpa->donor
+                            ),
+                            true
+                        ),
+                        $this->container->get(LoggerInterface::class),
+                    ),
                     'caseSubtype'          => $this->lpa->caseSubtype,
                     'activationKeyDueDate' => $activationKeyDueDate,
                 ],
-                $ex->getAdditionalData()
+                $ex->getAdditionalData(),
             );
             return;
         }
@@ -2733,13 +2704,20 @@ public function iRequestForANewActivationKeyAgain(): void
         $response    = $addOlderLpa->validateRequest($this->userId, $data);
 
         $expectedLpaArray = json_decode(json_encode($this->lpa), true);
-        $expectedLpa      = new \App\Service\Lpa\SiriusLpa($expectedLpaArray);
+        $expectedLpa      = new SiriusLpa($expectedLpaArray, $this->container->get(LoggerInterface::class));
         $expectedResponse = new AccessForAllValidation(
             new ActorMatch(
-                new SiriusPerson(json_decode(json_encode($this->lpa->donor), true)),
+                new SiriusPerson(
+                    json_decode(
+                        json_encode($this->lpa->donor),
+                        true
+                    ),
+                    $this->container->get(LoggerInterface::class)
+                ),
                 'donor',
-                $this->lpa->uId),
-                $expectedLpa,
+                $this->lpa->uId,
+            ),
+            $expectedLpa,
             null
         );
 

diff --git a/service-api/app/src/App/src/DataAccess/ApiGateway/SiriusLpas.php b/service-api/app/src/App/src/DataAccess/ApiGateway/SiriusLpas.php
@@ -141,6 +141,7 @@ public function lookup(array $uids): array
                         $results[$uid] = new Lpa(
                             new SiriusLpa(
                                 $this->sanitiser->sanitise($response),
+                                $this->logger,
                             ),
                             new DateTimeImmutable($result->getHeaderLine('Date'))
                         );

diff --git a/service-api/app/src/App/src/Service/Lpa/SiriusLpa.php b/service-api/app/src/App/src/Service/Lpa/SiriusLpa.php
@@ -12,6 +12,7 @@
 use ArrayAccess;
 use IteratorAggregate;
 use JsonSerializable;
+use Psr\Log\LoggerInterface;
 use Traversable;
 
 /**
@@ -29,7 +30,7 @@ class SiriusLpa implements
 {
     use SiriusHasActorTrait;
 
-    public function __construct(private array $lpa)
+    public function __construct(private array $lpa, private LoggerInterface $logger)
     {
         if ($this->lpa['donor'] !== null) {
             $donorAsSiriusPerson = $this->convertToSiriusPerson($this->lpa['donor']);
@@ -68,7 +69,7 @@ private function convertToSiriusPerson(SiriusPerson|array $entity): SiriusPerson
     {
         return $entity instanceof SiriusPerson
             ? $entity
-            : new SiriusPerson($entity);
+            : new SiriusPerson($entity, $this->logger);
     }
 
     private function getTrustCorporations(): array
@@ -78,16 +79,34 @@ private function getTrustCorporations(): array
 
     public function offsetExists(mixed $offset): bool
     {
+        $trace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 1);
+        $this->logger->debug(
+            'Use of SiriusLpa object as array (exists) in file '
+            . $trace[0]['file'] . ' on line ' . $trace[0]['line']
+        );
+
         return isset($this->lpa[$offset]);
     }
 
     public function offsetGet(mixed $offset): mixed
     {
+        $trace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 1);
+        $this->logger->debug(
+            'Use of SiriusLpa object as array (getter) in file '
+            . $trace[0]['file'] . ' on line ' . $trace[0]['line']
+        );
+
         return $this->lpa[$offset];
     }
 
     public function offsetSet(mixed $offset, mixed $value): void
     {
+        $trace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 1);
+        $this->logger->debug(
+            'Use of SiriusLpa object as array (setter) in file '
+            . $trace[0]['file'] . ' on line ' . $trace[0]['line']
+        );
+
         $this->lpa[$offset] = $value;
     }
 

diff --git a/service-api/app/src/App/src/Service/Lpa/SiriusPerson.php b/service-api/app/src/App/src/Service/Lpa/SiriusPerson.php
@@ -13,6 +13,7 @@
 use DateTimeInterface;
 use IteratorAggregate;
 use JsonSerializable;
+use Psr\Log\LoggerInterface;
 use Traversable;
 
 /**
@@ -28,7 +29,7 @@ class SiriusPerson implements
     IteratorAggregate,
     JsonSerializable
 {
-    public function __construct(private array $person)
+    public function __construct(private array $person, private LoggerInterface $logger)
     {
     }
 
@@ -87,16 +88,34 @@ public function getDob(): DateTimeInterface
 
     public function offsetExists(mixed $offset): bool
     {
+        $trace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 1);
+        $this->logger->debug(
+            'Use of SiriusPerson object as array (exists) in file '
+            . $trace[0]['file'] . ' on line ' . $trace[0]['line']
+        );
+
         return isset($this->person[$offset]);
     }
 
     public function offsetGet(mixed $offset): mixed
     {
+        $trace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 1);
+        $this->logger->debug(
+            'Use of SiriusPerson object as array (getter) in file '
+            . $trace[0]['file'] . ' on line ' . $trace[0]['line']
+        );
+
         return $this->person[$offset];
     }
 
     public function offsetSet(mixed $offset, mixed $value): void
     {
+        $trace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 1);
+        $this->logger->debug(
+            'Use of SiriusPerson object as array (setter) in file '
+            . $trace[0]['file'] . ' on line ' . $trace[0]['line']
+        );
+
         $this->person[$offset] = $value;
     }
 

diff --git a/service-api/app/test/AppTest/Service/ActorCodes/ActorCodeServiceTest.php b/service-api/app/test/AppTest/Service/ActorCodes/ActorCodeServiceTest.php
@@ -215,6 +215,7 @@ private function initValidParameterSet(): array
             [
                 'uId' => $testUid,
             ],
+            $this->loggerProphecy->reveal(),
         );
 
         $mockActor = new LpaActor(

diff --git a/...ice-api/app/test/AppTest/Service/ActorCodes/Validation/CodesApiValidationStrategyTest.php b/...ice-api/app/test/AppTest/Service/ActorCodes/Validation/CodesApiValidationStrategyTest.php
@@ -71,6 +71,7 @@ public function it_will_validate_a_code(): void
                 [
                     'uId' => 'lpa-uid',
                 ],
+                $this->loggerProphecy->reveal(),
             ),
             new DateTime('now')
         );
@@ -190,6 +191,7 @@ public function it_wont_validate_a_code_with_a_mismatched_actor(): void
                 [
                     'uId' => 'lpa-uid',
                 ],
+                $this->loggerProphecy->reveal(),
             ),
             new DateTime('now')
         );
@@ -230,6 +232,7 @@ public function it_wont_validate_a_code_with_a_bad_dob(): void
                 [
                     'uId' => 'lpa-uid',
                 ],
+                $this->loggerProphecy->reveal(),
             ),
             new DateTime('now')
         );
@@ -342,6 +345,7 @@ public function it_wont_validate_a_code_with_a_bad_dob_for_trust_corporation():
                     ],
                     'uId'   => 'lpa-uid',
                 ],
+                $this->loggerProphecy->reveal(),
             ),
             new DateTime('now')
         );
-Original file line number
+Diff line change
@@ Expand Up / @@ -215,6 +215,7 @@ private function initValidParameterSet(): array @@
                 [
                     'uId' => $testUid,
                 ],
+                $this->loggerProphecy->reveal(),
             );
             $mockActor = new LpaActor(
@@ Expand Down @@