From 1e8cf3cbc9f3d372e5187da04683eb411e676223 Mon Sep 17 00:00:00 2001
From: Frederik Bosch <f.bosch@genkgo.nl>
Date: Tue, 29 Sep 2020 15:46:02 +0200
Subject: [PATCH] using immutable objects for a stream result is a bad idea
because of gc cycles (#86)
---
src/Protocol/Imap/Response/AggregateResponse.php | 16 +++++++---------
.../CommandContinuationRequestResponse.php | 5 ++---
src/Protocol/Imap/Response/TaggedResponse.php | 5 ++---
src/Protocol/Imap/Response/UntaggedResponse.php | 5 ++---
.../Imap/Response/AggregateResponseTest.php | 4 ++--
.../CommandContinuationRequestResponseTest.php | 4 ++--
.../Imap/Response/TaggedResponseTest.php | 4 ++--
.../Imap/Response/UntaggedResponseTest.php | 4 ++--
8 files changed, 21 insertions(+), 26 deletions(-)
diff --git a/src/Protocol/Imap/Response/AggregateResponse.php b/src/Protocol/Imap/Response/AggregateResponse.php
index 9d1c928..3138812 100644
--- a/src/Protocol/Imap/Response/AggregateResponse.php
+++ b/src/Protocol/Imap/Response/AggregateResponse.php
@@ -106,41 +106,39 @@ final class AggregateResponse implements \IteratorAggregate
*/
public function withLine(string $line): AggregateResponse
{
- $clone = clone $this;
-
switch (\substr($line, 0, 2)) {
case '+ ':
- $clone->lines[] = new CommandContinuationRequestResponse(
+ $this->lines[] = new CommandContinuationRequestResponse(
\substr($line, 2)
);
break;
case '* ':
- $clone->lines[] = new UntaggedResponse(
+ $this->lines[] = new UntaggedResponse(
\substr($line, 2)
);
break;
default:
try {
- $clone->lines[] = new TaggedResponse(
+ $this->lines[] = new TaggedResponse(
$this->tag,
$this->tag->extractBodyFromLine($line)
);
} catch (\InvalidArgumentException $e) {
- if (empty($clone->lines)) {
+ if (empty($this->lines)) {
throw new \UnexpectedValueException(
'Expected line to begin with +, * or tag. Got: ' . $line
);
}
- $keys = \array_keys($clone->lines);
+ $keys = \array_keys($this->lines);
$lastKey = \end($keys);
- $clone->lines[$lastKey] = $clone->lines[$lastKey]->withAddedBody($line);
+ $this->lines[$lastKey]->withAddedBody($line);
}
break;
}
- return $clone;
+ return $this;
}
/**
diff --git a/src/Protocol/Imap/Response/CommandContinuationRequestResponse.php b/src/Protocol/Imap/Response/CommandContinuationRequestResponse.php
index 671d503..1b224a5 100644
--- a/src/Protocol/Imap/Response/CommandContinuationRequestResponse.php
+++ b/src/Protocol/Imap/Response/CommandContinuationRequestResponse.php
@@ -35,9 +35,8 @@ final class CommandContinuationRequestResponse implements ResponseInterface
*/
public function withAddedBody(string $data): ResponseInterface
{
- $clone = clone $this;
- $clone->line .= $data;
- return $clone;
+ $this->line .= $data;
+ return $this;
}
/**
diff --git a/src/Protocol/Imap/Response/TaggedResponse.php b/src/Protocol/Imap/Response/TaggedResponse.php
index 6ecef37..b0eee8d 100644
--- a/src/Protocol/Imap/Response/TaggedResponse.php
+++ b/src/Protocol/Imap/Response/TaggedResponse.php
@@ -43,9 +43,8 @@ final class TaggedResponse implements ResponseInterface
*/
public function withAddedBody(string $data): ResponseInterface
{
- $clone = clone $this;
- $clone->line .= $data;
- return $clone;
+ $this->line .= $data;
+ return $this;
}
/**
diff --git a/src/Protocol/Imap/Response/UntaggedResponse.php b/src/Protocol/Imap/Response/UntaggedResponse.php
index 113c5f4..f0c6811 100644
--- a/src/Protocol/Imap/Response/UntaggedResponse.php
+++ b/src/Protocol/Imap/Response/UntaggedResponse.php
@@ -40,9 +40,8 @@ final class UntaggedResponse implements ResponseInterface
*/
public function withAddedBody(string $data): ResponseInterface
{
- $clone = clone $this;
- $clone->line .= $data;
- return $clone;
+ $this->line .= $data;
+ return $this;
}
/**
diff --git a/test/Unit/Protocol/Imap/Response/AggregateResponseTest.php b/test/Unit/Protocol/Imap/Response/AggregateResponseTest.php
index a6a5567..645678b 100644
--- a/test/Unit/Protocol/Imap/Response/AggregateResponseTest.php
+++ b/test/Unit/Protocol/Imap/Response/AggregateResponseTest.php
@@ -27,10 +27,10 @@ final class AggregateResponseTest extends AbstractTestCase
/**
* @test
*/
- public function it_is_immutable(): void
+ public function it_is_mutable(): void
{
$response = new AggregateResponse(Tag::fromNonce(1));
- $this->assertNotSame($response, $response->withLine('* Untagged'));
+ $this->assertSame($response, $response->withLine('* Untagged'));
}
/**
diff --git a/test/Unit/Protocol/Imap/Response/CommandContinuationRequestResponseTest.php b/test/Unit/Protocol/Imap/Response/CommandContinuationRequestResponseTest.php
index 36d1ac3..2f7e348 100644
--- a/test/Unit/Protocol/Imap/Response/CommandContinuationRequestResponseTest.php
+++ b/test/Unit/Protocol/Imap/Response/CommandContinuationRequestResponseTest.php
@@ -24,10 +24,10 @@ final class CommandContinuationRequestResponseTest extends AbstractTestCase
/**
* @test
*/
- public function it_is_immutable(): void
+ public function it_is_mutable(): void
{
$response = new CommandContinuationRequestResponse('OK successful');
- $this->assertNotSame($response, $response->withAddedBody('body'));
+ $this->assertSame($response, $response->withAddedBody('body'));
}
/**
diff --git a/test/Unit/Protocol/Imap/Response/TaggedResponseTest.php b/test/Unit/Protocol/Imap/Response/TaggedResponseTest.php
index 7a4aa20..15c2ba4 100644
--- a/test/Unit/Protocol/Imap/Response/TaggedResponseTest.php
+++ b/test/Unit/Protocol/Imap/Response/TaggedResponseTest.php
@@ -25,10 +25,10 @@ final class TaggedResponseTest extends AbstractTestCase
/**
* @test
*/
- public function it_is_immutable(): void
+ public function it_is_mutable(): void
{
$response = new TaggedResponse(Tag::fromNonce(1), 'OK successful');
- $this->assertNotSame($response, $response->withAddedBody('body'));
+ $this->assertSame($response, $response->withAddedBody('body'));
}
/**
diff --git a/test/Unit/Protocol/Imap/Response/UntaggedResponseTest.php b/test/Unit/Protocol/Imap/Response/UntaggedResponseTest.php
index 8462b17..aff03b0 100644
--- a/test/Unit/Protocol/Imap/Response/UntaggedResponseTest.php
+++ b/test/Unit/Protocol/Imap/Response/UntaggedResponseTest.php
@@ -24,10 +24,10 @@ final class UntaggedResponseTest extends AbstractTestCase
/**
* @test
*/
- public function it_is_immutable(): void
+ public function it_is_mutable(): void
{
$response = new UntaggedResponse('OK successful');
- $this->assertNotSame($response, $response->withAddedBody('body'));
+ $this->assertSame($response, $response->withAddedBody('body'));
}
/**
--
GitLab