Repository: brefphp/bref (language: php)

Issue:
Make handlers more easily testable by removing `final` keyword
Reported by @marcguyer on Slack.

### Problem

Typed events (like `SqsEvent`) and `Context` are all `final`.

That was so that we could add any method without fearing breaking BC.

However, it makes mocking in tests very hard. You have to do create the payload manually, for example:

```php
$handler->handleSqs(
		$sqsEvent = new SqsEvent(['Records' => [[
			'eventSource' => 'aws:sqs',
			'body' => '{"eventRequestId":999,"startingState":"event_planned","targetState":"event_canceled"}',
		]]]),
		new Context('', intval(microtime(true) * 1000) + 6000, '', '')
);
```

Removing `final` would allow using `$this->createMock()`.

### Proposed solution

Remove `final` on these classes, but **document** in phpdoc that these classes should not be extended with new methods (no BC promise). They are "final" in spirit, but not in practice to allow mocking.


Discussion / hints:
Alternative approach: adding helper methods to quickly create instances without having to create a fake Lambda payload by hand.

Here's an example with just the `Context` class: https://github.com/brefphp/bref/pull/1398
Note: the approach above is great for `Context` I think, but is much more work to apply (and apply consistently) across all typed events.

It turns out `@final` is enforced by Phpstan and Psalm (https://phpstan.org/r/80a438f2-775f-4c79-b0e0-624bd17dd4f2), so relying on this annotation and removing `final` could work!
The classes you mention here should never be mocked. Mocks serve a controlled and manipulated behaviour. Those classes are, as far as I can tell, value objects therefore with a behavior that do not rely on external nor internal states.

Fake helpers are the correct way to go.

`@final` keyword enforced by SA works, but hides a technical debt under the carpet: fake helpers should be easy to create and maintain; if they aren't, I'd say there's a smell somewhere in the code architecture.
@Slamdunk I'm not really interested in debating generic theoretical ideas, because in my experience these are never useful to solve a specific problem.

I'm more interested in solving the problem here specifically. And the problem is, these typed handlers are hard to write tests for.

How can we solve that problem? It may be by making it easier to use `createMock`, it may be via another solution. I'd rather discuss concrete solutions rather than discuss abstract programming principles. Because "fake helpers" handwaves a lot of how we can make that happen in a realistic and practical way both for maintainers and users.
I prepared an option via a factory here: https://github.com/brefphp/bref/pull/1399
@mnapoli It seems my communication has been received by you as a theoretical debate: that wasn't my intention.
I've seen your tweet, I've seen you questioning yourself on the correct approach and asking the community for a feedback, so I gave you mine.

I would have completely avoided my (sparse) contribution if you would just have asked for the quickest workaround.
@Slamdunk  Understood, let's ignore whether or not its theoretical and move forward. We still aren't closer to a concrete example that improves the lives of users compared to what they can do today. That's what I'm interested in discussing.

Today it's mildly inconvenient. How can we make that experience better.
I first ran into some difficulty when writing tests to assert some "approaching" timeout behaviors. Due to the temporal nature of this method:

https://github.com/brefphp/bref/blob/master/src/Context/Context.php#L43-L46

... I wanted to mock it... to say something like `Context::getRemainingTimeInMillis() will return 10`... but the `final` keyword prevents that of course. Instead, we need to do this to end up getting a "real" value that fits into our test run window...

```php
new Context('', intval(microtime(true) * 1000) + 6000, '', '')
```


I encountered this as well. I am using [dg/bypass-finals](https://github.com/dg/bypass-finals) to workaround this.

Below is my Pest test file:

```php
<?php

use App\Handlers\KinesisEventHandler;
use App\Jobs\TestJob;
use Bref\Context\Context;
use Bref\Event\Kinesis\KinesisEvent;
use Bref\Event\Kinesis\KinesisRecord;
use Illuminate\Support\Facades\Bus;
use Mockery\MockInterface;

beforeEach(function () {
    DG\BypassFinals::enable();
});

test('processes kinesis event', function () {
    $record = $this->mock(KinesisRecord::class, function (MockInterface $mock) {
        $contents = file_get_contents('tests/Resources/kinesis/event.json');
        $data = json_decode($contents, true);

        $mock
            ->shouldReceive('getData')
            ->andReturn($data);
    });

    $event = $this->mock(KinesisEvent::class, function (MockInterface $mock) use ($record) {
        $mock
            ->shouldReceive('getRecords')
            ->andReturn([$record]);
    });

    $context = $this->mock(Context::class);

    $handler = app(KinesisEventHandler::class);

    Bus::fake();

    $handler->handleKinesis($event, $context);

    Bus::assertDispatched(function (TestJob $job) {
        return $job->parameter == 1;
    });
});
```

Fix patch:
diff --git a/src/Event/ApiGateway/WebsocketEvent.php b/src/Event/ApiGateway/WebsocketEvent.php
index e9bf2e288..55c62cea3 100644
--- a/src/Event/ApiGateway/WebsocketEvent.php
+++ b/src/Event/ApiGateway/WebsocketEvent.php
@@ -7,8 +7,10 @@
 
 /**
  * Represents a Lambda event when Lambda is invoked by ApiGateway websocket route.
+ *
+ * @final
  */
-final class WebsocketEvent implements LambdaEvent
+class WebsocketEvent implements LambdaEvent
 {
     private array $event;
     private string $routeKey;
diff --git a/src/Event/DynamoDb/DynamoDbEvent.php b/src/Event/DynamoDb/DynamoDbEvent.php
index 5e3762338..4a0d15a8c 100644
--- a/src/Event/DynamoDb/DynamoDbEvent.php
+++ b/src/Event/DynamoDb/DynamoDbEvent.php
@@ -8,8 +8,10 @@
 
 /**
  * Represents a Lambda event when Lambda is invoked by DynamoDB Streams.
+ *
+ * @final
  */
-final class DynamoDbEvent implements LambdaEvent
+class DynamoDbEvent implements LambdaEvent
 {
     private array $event;
 
diff --git a/src/Event/DynamoDb/DynamoDbRecord.php b/src/Event/DynamoDb/DynamoDbRecord.php
index d89815e66..b5c2b6cb0 100644
--- a/src/Event/DynamoDb/DynamoDbRecord.php
+++ b/src/Event/DynamoDb/DynamoDbRecord.php
@@ -4,7 +4,10 @@
 
 use InvalidArgumentException;
 
-final class DynamoDbRecord
+/**
+ * @final
+ */
+class DynamoDbRecord
 {
     private array $record;
 
diff --git a/src/Event/EventBridge/EventBridgeEvent.php b/src/Event/EventBridge/EventBridgeEvent.php
index 2ecdeffc2..dde573a8e 100644
--- a/src/Event/EventBridge/EventBridgeEvent.php
+++ b/src/Event/EventBridge/EventBridgeEvent.php
@@ -8,8 +8,10 @@
 
 /**
  * Represents a Lambda event when Lambda is invoked by EventBridge.
+ *
+ * @final
  */
-final class EventBridgeEvent implements LambdaEvent
+class EventBridgeEvent implements LambdaEvent
 {
     private array $event;
 
diff --git a/src/Event/Kafka/KafkaEvent.php b/src/Event/Kafka/KafkaEvent.php
index 4d7d61dfd..99615eb1f 100644
--- a/src/Event/Kafka/KafkaEvent.php
+++ b/src/Event/Kafka/KafkaEvent.php
@@ -6,7 +6,10 @@
 use Bref\Event\InvalidLambdaEvent;
 use InvalidArgumentException;
 
-final class KafkaEvent implements Event\LambdaEvent
+/**
+ * @final
+ */
+class KafkaEvent implements Event\LambdaEvent
 {
     private array $event;
 
diff --git a/src/Event/Kafka/KafkaHandler.php b/src/Event/Kafka/KafkaHandler.php
index 764694ff4..d748a36c0 100644
--- a/src/Event/Kafka/KafkaHandler.php
+++ b/src/Event/Kafka/KafkaHandler.php
@@ -10,7 +10,7 @@ abstract class KafkaHandler implements Handler
     abstract public function handleKafka(KafkaEvent $event, Context $context): void;
 
     /** {@inheritDoc} */
-    final public function handle($event, Context $context)
+    public function handle($event, Context $context)
     {
         $this->handleKafka(new KafkaEvent($event), $context);
     }
diff --git a/src/Event/Kafka/KafkaRecord.php b/src/Event/Kafka/KafkaRecord.php
index bf4e5e466..4c97c3d1a 100644
--- a/src/Event/Kafka/KafkaRecord.php
+++ b/src/Event/Kafka/KafkaRecord.php
@@ -4,7 +4,10 @@
 
 use InvalidArgumentException;
 
-final class KafkaRecord
+/**
+ * @final
+ */
+class KafkaRecord
 {
     private array $record;
 
diff --git a/src/Event/Kinesis/KinesisEvent.php b/src/Event/Kinesis/KinesisEvent.php
index 758b2dfbf..cfd035740 100644
--- a/src/Event/Kinesis/KinesisEvent.php
+++ b/src/Event/Kinesis/KinesisEvent.php
@@ -6,7 +6,10 @@
 use Bref\Event\LambdaEvent;
 use InvalidArgumentException;
 
-final class KinesisEvent implements LambdaEvent
+/**
+ * @final
+ */
+class KinesisEvent implements LambdaEvent
 {
     private array $event;
 
diff --git a/src/Event/Kinesis/KinesisRecord.php b/src/Event/Kinesis/KinesisRecord.php
index f73b13f17..ca45a1513 100644
--- a/src/Event/Kinesis/KinesisRecord.php
+++ b/src/Event/Kinesis/KinesisRecord.php
@@ -5,7 +5,10 @@
 use DateTimeImmutable;
 use InvalidArgumentException;
 
-final class KinesisRecord
+/**
+ * @final
+ */
+class KinesisRecord
 {
     private array $record;
 
diff --git a/src/Event/S3/S3Event.php b/src/Event/S3/S3Event.php
index 6496639a6..672dfdd6a 100644
--- a/src/Event/S3/S3Event.php
+++ b/src/Event/S3/S3Event.php
@@ -10,8 +10,10 @@
  * Represents an event when Lambda is invoked by S3.
  *
  * @see https://docs.aws.amazon.com/AmazonS3/latest/dev/notification-content-structure.html
+ *
+ * @final
  */
-final class S3Event implements LambdaEvent
+class S3Event implements LambdaEvent
 {
     private array $event;
 
diff --git a/src/Event/S3/S3Record.php b/src/Event/S3/S3Record.php
index e748abd36..e558ee910 100644
--- a/src/Event/S3/S3Record.php
+++ b/src/Event/S3/S3Record.php
@@ -7,8 +7,10 @@
 
 /**
  * @see https://docs.aws.amazon.com/AmazonS3/latest/dev/notification-content-structure.html
+ *
+ * @final
  */
-final class S3Record
+class S3Record
 {
     private array $record;
 
diff --git a/src/Event/Sns/SnsEvent.php b/src/Event/Sns/SnsEvent.php
index 14ffbe5ea..5ed239517 100644
--- a/src/Event/Sns/SnsEvent.php
+++ b/src/Event/Sns/SnsEvent.php
@@ -8,8 +8,10 @@
 
 /**
  * Represents a Lambda event when Lambda is invoked by SNS.
+ *
+ * @final
  */
-final class SnsEvent implements LambdaEvent
+class SnsEvent implements LambdaEvent
 {
     private array $event;
 
diff --git a/src/Event/Sns/SnsRecord.php b/src/Event/Sns/SnsRecord.php
index d860a4b27..919715ea8 100644
--- a/src/Event/Sns/SnsRecord.php
+++ b/src/Event/Sns/SnsRecord.php
@@ -9,8 +9,10 @@
  * Represents a SNS message record.
  *
  * For more information about each field, see https://docs.aws.amazon.com/sns/latest/api/API_Publish.html
+ *
+ * @final
  */
-final class SnsRecord
+class SnsRecord
 {
     private array $record;
 
diff --git a/src/Event/Sqs/SqsEvent.php b/src/Event/Sqs/SqsEvent.php
index a120279b9..1ae2c0d6e 100644
--- a/src/Event/Sqs/SqsEvent.php
+++ b/src/Event/Sqs/SqsEvent.php
@@ -8,8 +8,10 @@
 
 /**
  * Represents a Lambda event when Lambda is invoked by SQS.
+ *
+ * @final
  */
-final class SqsEvent implements LambdaEvent
+class SqsEvent implements LambdaEvent
 {
     private array $event;
 
diff --git a/src/Event/Sqs/SqsRecord.php b/src/Event/Sqs/SqsRecord.php
index 6398fbcab..4c9db8058 100644
--- a/src/Event/Sqs/SqsRecord.php
+++ b/src/Event/Sqs/SqsRecord.php
@@ -4,7 +4,10 @@
 
 use InvalidArgumentException;
 
-final class SqsRecord
+/**
+ * @final
+ */
+class SqsRecord
 {
     private array $record;
 


Test patch: