From 84a81b73aa5684a398af05c88ebd30b3225c2978 Mon Sep 17 00:00:00 2001
From: Stein Magne Bjorklund <steinmb@smbjorklund.com>
Date: Mon, 13 Sep 2021 13:07:18 +0200
Subject: [PATCH] Make Utils::getBestContentType() faster

- Always run and should be fast.
- Introduce tests to make sure I actually work.
- Stop method from using global states to make it testable and safe.
---
 classes/Utils.php   | 36 ++++++++++++++++++++++----------
 index.php           |  2 +-
 phpunit.xml         | 26 +++++++++++++++++++++++
 tests/UtilsTest.php | 51 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 103 insertions(+), 12 deletions(-)
 create mode 100644 phpunit.xml
 create mode 100644 tests/UtilsTest.php

diff --git a/classes/Utils.php b/classes/Utils.php
index 01fc9b90..d12752ce 100644
--- a/classes/Utils.php
+++ b/classes/Utils.php
@@ -136,27 +136,39 @@ class Utils
         return $extension;
     }
 
-    public static function getBestContentType($accept_string)
+    /**
+     * header from the current request, if there is one.
+     *
+     * @param $accept_string
+     *   Contents of the Accept: header from the current request.
+     * @param array $configuration
+     * @return int|string
+     */
+    public static function getBestContentType($accept_string, array $configuration)
     {
-        global $conf;
         $a = explode(",", $accept_string);
-        $b = array();
+        if ($a[0] === 'text/html' || $a[0] === 'text/css' || $a[0] === '*/*') {
+            return 'text/html';
+        }
+
+        $b = [];
         foreach ($a as $v) {
-            foreach ($conf['http_accept'] as $formatTypeArray) {
-                if (strstr($v, ";")) {
-                    $aux = explode(";", $v);
+            foreach ($configuration as $formatTypeArray) {
+                if (strpos($v, ";") !== false) {
+                    $aux = explode(';', $v);
                     $aux[0] = trim($aux[0]);
-                    if (in_array($aux[0], $formatTypeArray)) {
-                        $b[$aux[0]] = floatval(trim(str_replace("q=", "", $aux[1])));
+                    if (in_array($aux[0], $formatTypeArray, true)) {
+                        $b[$aux[0]] = (float)trim(str_replace("q=", "", $aux[1]));
                     }
                 } else {
                     $value = trim($v);
-                    if (in_array($value, $formatTypeArray)) {
+                    if (in_array($value, $formatTypeArray, true)) {
                         $b[$value] = 1.0;
                     }
                 }
             }
         }
+
         $a = $b;
         arsort($a);
         $ct = 'text/html';
@@ -164,9 +176,11 @@ class Utils
             $ct = $k;
             break;
         }
-        if ($ct == null || $ct == "" || $ct == "*/*") {
-            $ct = 'text/html';
+
+        if ($ct === null || $ct === "" || $ct === "*/*") {
+            return 'text/html';
         }
+
         return $ct;
     }
 
diff --git a/index.php b/index.php
index 971b798c..5a3b13be 100644
--- a/index.php
+++ b/index.php
@@ -50,7 +50,7 @@ $firstResults = array();
 $endpoints = array();
 $endpoints['local'] = new Endpoint($conf['endpoint']['local'], $conf['endpointParams']['config']);
 
-$acceptContentType = Utils::getBestContentType($_SERVER['HTTP_ACCEPT']);
+$acceptContentType = Utils::getBestContentType($_SERVER['HTTP_ACCEPT'], $conf['http_accept']);
 $extension = Utils::getExtension($acceptContentType); 
 
 
diff --git a/phpunit.xml b/phpunit.xml
new file mode 100644
index 00000000..bb0d4f69
--- /dev/null
+++ b/phpunit.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.5/phpunit.xsd"
+         bootstrap="vendor/autoload.php"
+         cacheResultFile=".phpunit.cache/test-results"
+         executionOrder="depends,defects"
+         forceCoversAnnotation="true"
+         beStrictAboutCoversAnnotation="true"
+         beStrictAboutOutputDuringTests="true"
+         beStrictAboutTodoAnnotatedTests="true"
+         failOnRisky="true"
+         failOnWarning="true"
+         verbose="true">
+    <testsuites>
+        <testsuite name="default">
+            <directory>tests</directory>
+        </testsuite>
+    </testsuites>
+
+    <coverage cacheDirectory=".phpunit.cache/code-coverage"
+              processUncoveredFiles="true">
+        <include>
+            <directory suffix=".php">classes</directory>
+        </include>
+    </coverage>
+</phpunit>
diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php
new file mode 100644
index 00000000..fd04e594
--- /dev/null
+++ b/tests/UtilsTest.php
@@ -0,0 +1,51 @@
+<?php declare(strict_types=1);
+
+use PHPUnit\Framework\TestCase;
+use uib\ub\loadspeakr\Utils;
+
+final class UtilsTest extends TestCase
+{
+
+  public function setUp(): void
+  {
+    parent::setUp();
+  }
+
+    /**
+     * Test Loadspeaker content negotiation.
+     *
+     * @covers \uib\ub\loadspeakr\Utils::getBestContentType
+     */
+  public function testGetBestContentType(): void
+  {
+      $http_accept = [
+        'html' => ['text/html'],
+        'rdf' => ['application/rdf+xml'],
+        'ttl' => [
+            'text/n3',
+            'application/x-turtle',
+            'application/turtle',
+            'text/turtle',
+            'application/rdf+turtle',
+        ],
+        'json' => [
+          'application/json',
+          'application/x-javascript',
+          'text/javascript',
+          'text/x-javascript',
+          'text/x-json',
+        ],
+        'nt' => ['text/plain'],
+      ];
+
+      self::assertSame('text/html', Utils::getBestContentType(null, $http_accept));
+      self::assertSame('text/html', Utils::getBestContentType('', $http_accept));
+      self::assertSame('text/html', Utils::getBestContentType('foo, bar', $http_accept));
+      self::assertSame('text/html', Utils::getBestContentType('text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8', $http_accept));
+      self::assertSame('text/html', Utils::getBestContentType('text/css,*/*;q=0.1', $http_accept));
+      self::assertSame('application/json', Utils::getBestContentType('json/txt,application/json,*/*;q=0.1', $http_accept));
+      self::assertSame('application/json', Utils::getBestContentType('json/txt, application/json, */* ;q=0.1', $http_accept));
+      self::assertSame('application/json', Utils::getBestContentType('json/txt,application/foo,*/*;q=0.1', $http_accept));
+  }
+
+}
-- 
GitLab