Merge 82d8722 into b179cc1

openzipkin · Jun 7, 2021 · ccbc409 · ccbc409
2 parents b179cc1 + 82d8722
commit ccbc409
Show file tree

Hide file tree

Showing 2 changed files with 4 additions and 72 deletions.
diff --git a/src/Zipkin/Reporters/Http.php b/src/Zipkin/Reporters/Http.php
@@ -9,18 +9,13 @@
 use Zipkin\Reporters\Http\CurlFactory;
 use Zipkin\Reporters\Http\ClientFactory;
 use Zipkin\Reporter;
-use Zipkin\Recording\Span;
 use Zipkin\Recording\ReadbackSpan;
-use TypeError;
 use RuntimeException;
 use Psr\Log\NullLogger;
 use Psr\Log\LoggerInterface;
-use InvalidArgumentException;
 
 final class Http implements Reporter
 {
-    private const EMPTY_ARG = 'empty_arg';
-
     public const DEFAULT_OPTIONS = [
         'endpoint_url' => 'http://localhost:9411/api/v2/spans',
     ];
@@ -66,40 +61,12 @@ final class Http implements Reporter
      * @param SpanSerializer $serializer
      */
     public function __construct(
-        $options = self::EMPTY_ARG,
-        $requesterFactory = null,
+        array $options = [],
+        ClientFactory $requesterFactory = null,
         LoggerInterface $logger = null,
         SpanSerializer $serializer = null
     ) {
-        // V1 signature did not make much sense as in 99% of the cases, you don't
-        // want to pass a ClientFactory to it but $options instead. There was a plan
-        // to change that in v2 but it would be a breaking change, hence we added this
-        // logic to accept new signature but keep compabitility with the old one.
-
-        if ($options === self::EMPTY_ARG) {
-            // means no arguments because first argument wasn't nullable in v1
-            $parsedOptions = [];
-        } elseif (\is_array($options) && (($requesterFactory instanceof ClientFactory) || $requesterFactory == null)) {
-            // means the intention of the first argument is the `options`
-            $parsedOptions = $options;
-        } elseif ($options instanceof ClientFactory && (\is_array($requesterFactory) || $requesterFactory === null)) {
-            // means the intention of the first argument is the `ClientFactory`
-            $parsedOptions = $requesterFactory ?? [];
-            $requesterFactory = $options;
-        } elseif ($options === null) {
-            $parsedOptions = $requesterFactory ?? [];
-            $requesterFactory = null;
-        } else {
-            throw new TypeError(
-                \sprintf(
-                    'Argument 1 passed to %s::__construct must be of type array, %s given',
-                    self::class,
-                    \gettype($options)
-                )
-            );
-        }
-
-        $this->options = \array_merge(self::DEFAULT_OPTIONS, $parsedOptions);
+        $this->options = \array_merge(self::DEFAULT_OPTIONS, $options);
         $this->clientFactory = $requesterFactory ?? CurlFactory::create();
         $this->logger = $logger ?? new NullLogger();
         $this->serializer = $serializer ?? new JsonV2Serializer();

diff --git a/tests/Unit/Reporters/HttpTest.php b/tests/Unit/Reporters/HttpTest.php
@@ -10,9 +10,9 @@
 use Zipkin\Endpoint;
 use TypeError;
 use Psr\Log\LoggerInterface;
+use Prophecy\PhpUnit\ProphecyTrait;
 use Prophecy\Argument;
 use PHPUnit\Framework\TestCase;
-use Prophecy\PhpUnit\ProphecyTrait;
 
 final class HttpTest extends TestCase
 {
@@ -21,41 +21,6 @@ final class HttpTest extends TestCase
     const PAYLOAD = '[{"id":"%s","traceId":"%s",'
         . '"timestamp":%d,"name":"test","localEndpoint":{"serviceName":""}}]';
 
-    public function testConstructorIsRetrocompatible()
-    {
-        $this->assertInstanceOf(Http::class, new Http());
-
-        // old constructor
-        $this->assertInstanceOf(Http::class, new Http(
-            null,
-            ['endpoint_url' => 'http://myzipkin:9411/api/v2/spans']
-        ));
-        $this->assertInstanceOf(Http::class, new Http(CurlFactory::create()));
-        $this->assertInstanceOf(Http::class, new Http(
-            CurlFactory::create(),
-            ['endpoint_url' => 'http://myzipkin:9411/api/v2/spans']
-        ));
-
-        // new constructor
-        $this->assertInstanceOf(Http::class, new Http(
-            ['endpoint_url' => 'http://localhost:9411/api/v2/spans']
-        ));
-        $this->assertInstanceOf(Http::class, new Http(
-            ['endpoint_url' => 'http://localhost:9411/api/v2/spans'],
-            CurlFactory::create()
-        ));
-
-        try {
-            new Http(1);
-            $this->fail('Expected the constructor to fail.');
-        } catch (TypeError $e) {
-            $this->assertEquals(
-                'Argument 1 passed to Zipkin\Reporters\Http::__construct must be of type array, integer given',
-                $e->getMessage()
-            );
-        }
-    }
-
     public function testHttpReporterSuccess()
     {
         $context = TraceContext::createAsRoot();