Merge pull request #512 from Philipp91/vop-minor

#477 Polish code for easier reuse in VoP implementation

Author: nemiah

lib/Fhp/BaseAction.php

@@ -243,10 +243,8 @@ final public function setRequestSegmentNumbers(array $requestSegmentNumbers)
         $this->requestSegmentNumbers = $requestSegmentNumbers;
     }
 
-    /**
-     * To be called only by the FinTs instance that executes this action.
-     */
-    final public function setTanRequest(?TanRequest $tanRequest)
+    /** To be called only by the FinTs instance that executes this action. */
+    final public function setTanRequest(?TanRequest $tanRequest): void
     {
         $this->tanRequest = $tanRequest;
     }

lib/Fhp/FinTs.php

@@ -201,7 +201,7 @@ public function __unserialize(array $data): void
      *
      * @throws \InvalidArgumentException
      */
-    public function loadPersistedInstance(string $persistedInstance)
+    public function loadPersistedInstance(string $persistedInstance): void
     {
         $unserialized = unserialize($persistedInstance);
         if (!is_array($unserialized) || count($unserialized) === 0) {
@@ -216,7 +216,7 @@ public function loadPersistedInstance(string $persistedInstance)
         }
     }
 
-    private function loadPersistedInstanceVersion2(array $data)
+    private function loadPersistedInstanceVersion2(array $data): void
     {
         list( // This should match persist().
             $this->bpd,
@@ -254,7 +254,7 @@ public function setLogger(LoggerInterface $logger): void
      * @param int $responseTimeout The number of seconds to wait before aborting a request to the bank server.
      * @noinspection PhpUnused
      */
-    public function setTimeouts(int $connectTimeout, int $responseTimeout)
+    public function setTimeouts(int $connectTimeout, int $responseTimeout): void
     {
         $this->options->timeoutConnect = $connectTimeout;
         $this->options->timeoutResponse = $responseTimeout;
@@ -304,34 +304,51 @@ public function login(): DialogInitialization
      * @throws ServerException When the server responds with a (FinTS-encoded) error message, which includes most things
      *     that can go wrong with the action itself, like wrong credentials, invalid IBANs, locked accounts, etc.
      */
-    public function execute(BaseAction $action)
+    public function execute(BaseAction $action): void
     {
         if ($this->dialogId === null && !($action instanceof DialogInitialization)) {
             throw new \RuntimeException('Need to login (DialogInitialization) before executing other actions');
         }
 
+        // Add the action's main request segments.
         $requestSegments = $action->getNextRequest($this->bpd, $this->upd);
-
         if (count($requestSegments) === 0) {
             return; // No request needed.
         }
+        $message = MessageBuilder::create()->add($requestSegments);
 
-        // Construct the full request message.
-        $message = MessageBuilder::create()->add($requestSegments); // This fills in the segment numbers.
+        // Add HKTAN for authentication if necessary.
         if (!($this->getSelectedTanMode() instanceof NoPsd2TanMode)) {
             if (($needTanForSegment = $action->getNeedTanForSegment()) !== null) {
                 $message->add(HKTANFactory::createProzessvariante2Step1(
                     $this->requireTanMode(), $this->selectedTanMedium, $needTanForSegment));
             }
         }
-        $request = $this->buildMessage($message, $this->getSelectedTanMode());
+
+        // Construct the request and tell the action about the segment numbers that were assigned.
+        $request = $this->buildMessage($message, $this->getSelectedTanMode()); // This fills in the segment numbers.
         $action->setRequestSegmentNumbers(array_map(function ($segment) {
             /* @var BaseSegment $segment */
             return $segment->getSegmentNumber();
         }, $requestSegments));
 
         // Execute the request.
         $response = $this->sendMessage($request);
+        $this->processServerResponse($action, $response);
+    }
+
+    /**
+     * Updates the state of this FinTs instance and of the `$action` based on the server's response.
+     * See {@link execute()} for more documentation on the possible outcomes.
+     * @param BaseAction $action The action for which the request was sent.
+     * @param Message $response The response we just got from the server.
+     * @throws CurlException When the connection fails in a layer below the FinTS protocol.
+     * @throws UnexpectedResponseException When the server responds with a valid but unexpected message.
+     * @throws ServerException When the server responds with a (FinTS-encoded) error message, which includes most things
+     *      that can go wrong with the action itself, like wrong credentials, invalid IBANs, locked accounts, etc.
+     */
+    private function processServerResponse(BaseAction $action, Message $response): void
+    {
         $this->readBPD($response);
 
         // Detect if the bank wants a TAN.
@@ -379,7 +396,7 @@ public function execute(BaseAction $action)
      * @throws ServerException When the server responds with a (FinTS-encoded) error message, which includes most things
      *     that can go wrong with the action itself, like wrong credentials, invalid IBANs, locked accounts, etc.
      */
-    public function submitTan(BaseAction $action, string $tan)
+    public function submitTan(BaseAction $action, string $tan): void
     {
         // Check the action's state.
         $tanRequest = $action->getTanRequest();
@@ -538,7 +555,7 @@ public function checkDecoupledSubmission(BaseAction $action): bool
      * from cached BPD/UPD upon the next {@link login()}, for instance.
      * @throws ServerException When closing the dialog fails.
      */
-    public function close()
+    public function close(): void
     {
         if ($this->dialogId !== null) {
             $this->endDialog();
@@ -552,7 +569,7 @@ public function close()
      * This can be called by the application using this library when it just restored this FinTs instance from the
      * persisted format after a long time, during which the session/dialog has most likely expired on the server side.
      */
-    public function forgetDialog()
+    public function forgetDialog(): void
     {
         $this->dialogId = null;
     }
@@ -573,7 +590,9 @@ public function getTanModes(): array
         $this->ensureTanModesAvailable();
         $result = [];
         foreach ($this->allowedTanModes as $tanModeId) {
-            if (!array_key_exists($tanModeId, $this->bpd->allTanModes)) continue;
+            if (!array_key_exists($tanModeId, $this->bpd->allTanModes)) {
+                continue;
+            }
             $result[$tanModeId] = $this->bpd->allTanModes[$tanModeId];
         }
         return $result;
@@ -624,7 +643,7 @@ public function getTanMedia($tanMode): array
      *     must be the value returned from {@link TanMedium::getName()} for one of the TAN media supported with that TAN
      *     mode. Use {@link getTanMedia()} to obtain a list of possible TAN media options.
      */
-    public function selectTanMode($tanMode, $tanMedium = null)
+    public function selectTanMode($tanMode, $tanMedium = null): void
     {
         if (!is_int($tanMode) && !($tanMode instanceof TanMode)) {
             throw new \InvalidArgumentException('tanMode must be an int or a TanMode');
@@ -664,7 +683,7 @@ public function getBpd(): BPD
      * @throws UnexpectedResponseException When the server does not send the BPD or close the dialog properly.
      * @throws ServerException When the server resopnds with an error.
      */
-    private function ensureBpdAvailable()
+    private function ensureBpdAvailable(): void
     {
         if ($this->bpd !== null) {
             return; // Nothing to do.
@@ -711,7 +730,7 @@ private function requireCredentials(): Credentials
      *     like it should according to the protocol, or when the dialog is not closed properly.
      * @throws ServerException When the server responds with an error.
      */
-    private function ensureTanModesAvailable()
+    private function ensureTanModesAvailable(): void
     {
         if ($this->allowedTanModes === null) {
             $this->ensureBpdAvailable();
@@ -730,7 +749,7 @@ private function ensureTanModesAvailable()
      *     dialog is not closed properly.
      * @throws ServerException When the server responds with an error.
      */
-    private function ensureSynchronized()
+    private function ensureSynchronized(): void
     {
         if ($this->kundensystemId === null) {
             $this->ensureBpdAvailable();
@@ -820,7 +839,7 @@ protected function newConnection(): Connection
     /**
      * Closes the physical connection, if necessary.
      */
-    private function disconnect()
+    private function disconnect(): void
     {
         if ($this->connection !== null) {
             $this->connection->disconnect();
@@ -834,7 +853,7 @@ private function disconnect()
      * @param Message $fakeResponseMessage A messsage that contains the response segments for this action.
      * @throws UnexpectedResponseException When the server responded with a valid but unexpected message.
      */
-    private function processActionResponse(BaseAction $action, Message $fakeResponseMessage)
+    private function processActionResponse(BaseAction $action, Message $fakeResponseMessage): void
     {
         $action->processResponse($fakeResponseMessage);
         if ($action instanceof DialogInitialization) {
@@ -864,7 +883,7 @@ private function processActionResponse(BaseAction $action, Message $fakeResponse
      *     properly.
      * @throws ServerException When the server responds with an error.
      */
-    private function executeWeakDialogInitialization(?string $hktanRef)
+    private function executeWeakDialogInitialization(?string $hktanRef): void
     {
         if ($this->dialogId !== null) {
             throw new \RuntimeException('Cannot init another dialog.');
@@ -905,7 +924,7 @@ private function readBPD(Message $response): bool
      * @throws ServerException When the server responds with an error instead of closing the dialog. This means that
      *     the connection is tainted and can probably not be used for another dialog.
      */
-    protected function endDialog(bool $isAnonymous = false)
+    protected function endDialog(bool $isAnonymous = false): void
     {
         if ($this->connection === null) {
             $this->dialogId = null;
@@ -943,7 +962,7 @@ protected function endDialog(bool $isAnonymous = false)
      * @param MessageBuilder $message The message to be built.
      * @param TanMode|null $tanMode Optionally a TAN mode that will be used when sending this message, defaults to 999
      *     (single step).
-     * @param string|null Optionally a TAN to sign this message with.
+     * @param string|null $tan Optionally a TAN to sign this message with.
      * @return Message The built message.
      */
     private function buildMessage(MessageBuilder $message, ?TanMode $tanMode = null, ?string $tan = null): Message

lib/Fhp/PaginateableAction.php

@@ -6,7 +6,6 @@
 use Fhp\Protocol\Message;
 use Fhp\Protocol\UnexpectedResponseException;
 use Fhp\Protocol\UPD;
-use Fhp\Segment\BaseSegment;
 use Fhp\Segment\HIRMS\Rueckmeldungscode;
 use Fhp\Segment\Paginateable;
 
@@ -79,7 +78,7 @@ public function hasMorePages(): bool
 
     public function processResponse(Message $response)
     {
-        if (($pagination = $response->findRueckmeldung(Rueckmeldungscode::PAGINATION)) !== null) {
+        if (($pagination = $response->findRueckmeldung(Rueckmeldungscode::AUFSETZPUNKT)) !== null) {
             if (count($pagination->rueckmeldungsparameter) !== 1) {
                 throw new UnexpectedResponseException("Unexpected pagination request: $pagination");
             }

lib/Fhp/Protocol/Message.php

@@ -300,14 +300,16 @@ public static function parse(string $rawMessage): Message
         $segments = Parser::parseSegments($rawMessage);
 
         // Message header and footer must always be there, or something went badly wrong.
-        if (!($segments[0] instanceof HNHBKv3)) {
-            throw new \InvalidArgumentException("Expected first segment to be HNHBK: $rawMessage");
-        }
-        if (!($segments[count($segments) - 1] instanceof HNHBSv1)) {
-            throw new \InvalidArgumentException("Expected last segment to be HNHBS: $rawMessage");
-        }
         $result->header = $segments[0];
         $result->footer = $segments[count($segments) - 1];
+        if (!($result->header instanceof HNHBKv3)) {
+            $actual = $result->header->getName();
+            throw new \InvalidArgumentException("Expected first segment to be HNHBK, but got $actual: $rawMessage");
+        }
+        if (!($result->footer instanceof HNHBSv1)) {
+            $actual = $result->footer->getName();
+            throw new \InvalidArgumentException("Expected last segment to be HNHBS, but got $actual: $rawMessage");
+        }
 
         // Check if there's an encryption header and "encrypted" data.
         // Section B.8 specifies that there are exactly 4 segments: HNHBK, HNVSK, HNVSD, HNHBS.

lib/Fhp/Segment/HIRMS/Rueckmeldungscode.php

@@ -77,7 +77,7 @@ public static function isError(int $code): bool
      * Tells the client that the response is incomplete and the request needs to be re-sent with the pagination token
      * ("Aufsetzpunkt") that is contained in the Rueckmeldung parameters.
      */
-    public const PAGINATION = 3040;
+    public const AUFSETZPUNKT = 3040;
 
     public const VOP_KEINE_NAMENSABWEICHUNG = 25;
 

lib/Tests/Fhp/FinTsPeer.php

@@ -13,10 +13,7 @@
  */
 class FinTsPeer extends FinTs
 {
-    /**
-     * @var Connection
-     */
-    public static $mockConnection;
+    public static ?Connection $mockConnection = null;
 
     public function __construct(FinTsOptions $options, ?Credentials $credentials)
     {
@@ -31,12 +28,12 @@ protected function newConnection(): Connection
     /**
      * @throws ServerException
      */
-    public function endDialog(bool $isAnonymous = false) // parent::endDialog() is protected
+    public function endDialog(bool $isAnonymous = false): void // parent::endDialog() is protected
     {
         parent::endDialog($isAnonymous);
     }
 
-    public function getDialogId()
+    public function getDialogId(): ?string
     {
         return $this->dialogId;
     }