Merge pull request #24 from swaggest/stable-rearrange

vearutop · web-flow · commit 203014d2d87a · 2019-09-26T13:36:16.000+02:00
Stable rearrange
diff --git a/.gitattributes b/.gitattributes
@@ -3,12 +3,14 @@
 *.dat           -text
 /tests export-ignore
 /stubs export-ignore
+/tools export-ignore
 /.gitattributes export-ignore
 /.gitignore export-ignore
+/.gitmodules export-ignore
 /.scrutinizer.yml export-ignore
 /.travis.yml export-ignore
 /.gitlab-ci.yml
 /phpunit.xml export-ignore
+/phpstan.neon export-ignore
 /changelog.md export-ignore
 /Makefile export-ignore
-/.gitlab-ci.yml
diff --git a/.gitignore b/.gitignore
@@ -1,4 +1,10 @@
 /vendor
 /.idea
 /composer.phar
-/clover.xml
+/clover.xml
+build
+/xhprof_report.*
+benchmark-result.json
+/coverage.xml
+/phpbench-candidate.xml
+/phpbench-master.xml
diff --git a/.travis.yml b/.travis.yml
@@ -1,7 +1,6 @@
 language: php
 php:
   - nightly
-  - hhvm
   - 7.3
   - 7.2
   - 7.1
@@ -16,26 +15,20 @@ dist: trusty
 ## Cache composer bits
 cache:
   directories:
-    - $HOME/.composer/cache
+    - $HOME/.cache/composer
 
-# execute any number of scripts before the test run, custom env's are available as variables
 before_script:
   - composer install --dev --no-interaction --prefer-dist
-  - if [[ $(phpenv version-name) =~ 7.2 ]] ; then test -f $HOME/.composer/cache/phpstan.phar || wget https://github.com/phpstan/phpstan/releases/download/0.9.2/phpstan.phar -O $HOME/.composer/cache/phpstan.phar; fi
-  - if [[ $(phpenv version-name) =~ 7.2 ]] ; then test -f $HOME/.composer/cache/ocular.phar || wget https://scrutinizer-ci.com/ocular.phar -O $HOME/.composer/cache/ocular.phar; fi
-  - if [[ $(phpenv version-name) =~ 7.2 ]] ; then test -f $HOME/.composer/cache/cctr || wget https://codeclimate.com/downloads/test-reporter/test-reporter-0.1.4-linux-amd64 -O $HOME/.composer/cache/cctr && chmod +x $HOME/.composer/cache/cctr; fi
-  - if [[ $(phpenv version-name) =~ 7.2 ]] ; then $HOME/.composer/cache/cctr before-build; fi
+  - if ! [[ $(phpenv version-name) =~ 7.3 ]] ; then phpenv config-rm xdebug.ini || true ; fi
 
 matrix:
   allow_failures:
-    - php: hhvm
     - php: nightly
   fast_finish: true
 
 script:
-  - ./vendor/bin/phpunit -v --configuration phpunit.xml --coverage-text --coverage-clover clover.xml
-  - if [[ $(phpenv version-name) =~ 7.2 ]] ; then php $HOME/.composer/cache/phpstan.phar analyze -l 7 ./src; fi
+  - if [[ $(phpenv version-name) =~ 7.3 ]] ; then make test-coverage; else make test; fi
+  - if [[ $(phpenv version-name) =~ 7.2 ]] ; then make lint bench bench-master bench-compare; fi
 
 after_script:
- - if [[ $(phpenv version-name) =~ 7.2 ]] ; then php $HOME/.composer/cache/ocular.phar code-coverage:upload --format=php-clover clover.xml; fi
- - if [[ $(phpenv version-name) =~ 7.2 ]] ; then $HOME/.composer/cache/cctr after-build --exit-code $TRAVIS_TEST_RESULT; fi
+  - if [[ $(phpenv version-name) =~ 7.3 ]] ; then bash <(curl -s https://codecov.io/bash); fi
diff --git a/Makefile b/Makefile
@@ -1,9 +1,32 @@
-phar:
-	composer require php-yaoi/php-yaoi:^1;composer install --no-dev;rm -rf tests/;rm ./json-diff;rm ./json-diff.tar.gz;phar-composer build;mv ./json-diff.phar ./json-diff;tar -zcvf ./json-diff.tar.gz ./json-diff;git reset --hard;composer install
+PHPSTAN_VERSION ?= 0.11.15
+PHPBENCH_VERSION ?= 0.16.10
 
-docker56-composer-update:
-	test -f ./composer.phar || wget https://getcomposer.org/composer.phar
-	docker run --rm -v $$(pwd):/code php:5.6-cli bash -c "apt-get update;apt-get install -y unzip;cd /code;php composer.phar update --prefer-source"
+deps:
+	@git submodule init && git submodule update
 
-docker56-test:
-	docker run --rm -v $$(pwd):/code -w /code php:5.6-cli php vendor/bin/phpunit
+lint:
+	@test -f ${HOME}/.cache/composer/phpstan-${PHPSTAN_VERSION}.phar || (mkdir -p ${HOME}/.cache/composer/ && wget https://github.com/phpstan/phpstan/releases/download/${PHPSTAN_VERSION}/phpstan.phar -O ${HOME}/.cache/composer/phpstan-${PHPSTAN_VERSION}.phar)
+	@php $$HOME/.cache/composer/phpstan-${PHPSTAN_VERSION}.phar analyze -l 7 -c phpstan.neon ./src
+
+docker-lint:
+	@docker run -v $$PWD:/app --rm phpstan/phpstan analyze -l 7 -c phpstan.neon ./src
+
+test:
+	@php -derror_reporting="E_ALL & ~E_DEPRECATED" vendor/bin/phpunit --configuration phpunit.xml
+
+test-coverage:
+	@php -derror_reporting="E_ALL & ~E_DEPRECATED" -dzend_extension=xdebug.so vendor/bin/phpunit --configuration phpunit.xml --coverage-text --coverage-clover=coverage.xml
+
+phpbench:
+	@test -f ${HOME}/.cache/composer/phpbench-${PHPBENCH_VERSION}.phar || (mkdir -p ${HOME}/.cache/composer/ && wget https://github.com/phpbench/phpbench/releases/download/${PHPBENCH_VERSION}/phpbench.phar -O ${HOME}/.cache/composer/phpbench-${PHPBENCH_VERSION}.phar)
+
+bench: phpbench
+	@php $$HOME/.cache/composer/phpbench-${PHPBENCH_VERSION}.phar run benchmarks --tag=candidate --progress=travis --bootstrap=vendor/autoload.php --revs=50 --iterations=5 --retry-threshold=3 --dump-file=phpbench-candidate.xml
+
+bench-master: phpbench
+	@git checkout --detach && git fetch origin '+refs/heads/master:refs/heads/master' && git checkout master -- ./src
+	@composer install --dev --no-interaction --prefer-dist
+	@php $$HOME/.cache/composer/phpbench-${PHPBENCH_VERSION}.phar run benchmarks --tag=master --progress=none --bootstrap=vendor/autoload.php --revs=50 --iterations=5 --retry-threshold=3 --dump-file=phpbench-master.xml
+
+bench-compare: phpbench
+	@php $$HOME/.cache/composer/phpbench-${PHPBENCH_VERSION}.phar report --file phpbench-master.xml --file phpbench-candidate.xml --report='generator: "table", cols: [ "set" ], compare: "tag", compare_fields: ["mean"], break: ["benchmark"]'
diff --git a/benchmarks/DiffBench.php b/benchmarks/DiffBench.php
@@ -0,0 +1,109 @@
+<?php
+
+use Swaggest\JsonDiff\JsonDiff;
+
+class DiffBench
+{
+    static $simpleOriginal;
+    static $simpleNew;
+
+    static $original;
+    static $new;
+
+    public function benchSimpleSkipPatch()
+    {
+        new JsonDiff(self::$simpleOriginal, self::$simpleNew, JsonDiff::REARRANGE_ARRAYS);
+    }
+
+    public function benchSimpleRearrange()
+    {
+        new JsonDiff(self::$simpleOriginal, self::$simpleNew, JsonDiff::REARRANGE_ARRAYS);
+    }
+
+    public function benchSkipPatch()
+    {
+        new JsonDiff(self::$original, self::$new, JsonDiff::REARRANGE_ARRAYS);
+    }
+
+    public function benchRearrange()
+    {
+        new JsonDiff(self::$original, self::$new, JsonDiff::REARRANGE_ARRAYS);
+    }
+
+    public function benchStopOnDiff()
+    {
+        new JsonDiff(self::$original, self::$new, JsonDiff::STOP_ON_DIFF);
+    }
+
+
+    static function init()
+    {
+        self::$simpleOriginal = (object)(array("root" => (object)array("a" => 1, "b" => 2)));
+        self::$simpleNew = (object)(array("root" => (object)array("b" => 3, "c" => 4)));
+        self::$original = json_decode(<<<'JSON'
+{
+  "key1": [
+    4,
+    1,
+    2,
+    3
+  ],
+  "key2": 2,
+  "key3": {
+    "sub0": 0,
+    "sub1": "a",
+    "sub2": "b"
+  },
+  "key4": [
+    {
+      "a": 1,
+      "b": true
+    },
+    {
+      "a": 2,
+      "b": false
+    },
+    {
+      "a": 3
+    }
+  ]
+}
+JSON
+        );
+
+        self::$new = json_decode(<<<'JSON'
+{
+  "key5": "wat",
+  "key1": [
+    5,
+    1,
+    2,
+    3
+  ],
+  "key4": [
+    {
+      "c": false,
+      "a": 2
+    },
+    {
+      "a": 1,
+      "b": true
+    },
+    {
+      "c": 1,
+      "a": 3
+    }
+  ],
+  "key3": {
+    "sub3": 0,
+    "sub2": false,
+    "sub1": "c"
+  }
+}
+JSON
+        );
+    }
+
+}
+
+DiffBench::init();
diff --git a/phpstan.neon b/phpstan.neon
diff --git a/src/JsonDiff.php b/src/JsonDiff.php
@@ -392,6 +392,11 @@ private function process($original, $new)
         return is_array($new) ? $newOrdered : (object)$newOrdered;
     }
 
+    /**
+     * @param array $original
+     * @param array $new
+     * @return array
+     */
     private function rearrangeArray(array $original, array $new)
     {
         $first = reset($original);
@@ -412,7 +417,7 @@ private function rearrangeArray(array $original, array $new)
 
             $keyIsUnique = true;
             $uniqueIdx = array();
-            foreach ($original as $item) {
+            foreach ($original as $idx => $item) {
                 if (!$item instanceof \stdClass) {
                     return $new;
                 }
@@ -430,7 +435,7 @@ private function rearrangeArray(array $original, array $new)
                     $keyIsUnique = false;
                     break;
                 }
-                $uniqueIdx[$value] = true;
+                $uniqueIdx[$value] = $idx;
             }
 
             if ($keyIsUnique) {
@@ -439,43 +444,54 @@ private function rearrangeArray(array $original, array $new)
             }
         }
 
-        if ($uniqueKey) {
-            $newIdx = array();
-            foreach ($new as $item) {
-                if (!$item instanceof \stdClass) {
-                    return $new;
-                }
+        if (!$uniqueKey) {
+            return $new;
+        }
 
-                if (!property_exists($item, $uniqueKey)) {
-                    return $new;
-                }
+        $newRearranged = [];
+        $changedItems = [];
 
-                $value = $item->$uniqueKey;
+        foreach ($new as $item) {
+            if (!$item instanceof \stdClass) {
+                return $new;
+            }
 
-                if ($value instanceof \stdClass || is_array($value)) {
-                    return $new;
-                }
+            if (!property_exists($item, $uniqueKey)) {
+                return $new;
+            }
 
-                if (isset($newIdx[$value])) {
-                    return $new;
-                }
+            $value = $item->$uniqueKey;
 
-                $newIdx[$value] = $item;
+            if ($value instanceof \stdClass || is_array($value)) {
+                return $new;
             }
 
-            $newRearranged = array();
-            foreach ($uniqueIdx as $key => $item) {
-                if (isset($newIdx[$key])) {
-                    $newRearranged [] = $newIdx[$key];
-                    unset($newIdx[$key]);
+
+            if (isset($uniqueIdx[$value])) {
+                $idx = $uniqueIdx[$value];
+                // Abandon rearrangement if key is not unique in new array.
+                if (isset($newRearranged[$idx])) {
+                    return $new;
                 }
+
+                $newRearranged[$idx] = $item;
+            } else {
+                $changedItems[] = $item;
             }
-            foreach ($newIdx as $item) {
-                $newRearranged [] = $item;
+
+            $newIdx[$value] = $item;
+        }
+
+        $idx = 0;
+        foreach ($changedItems as $item) {
+            while (array_key_exists($idx, $newRearranged)) {
+                $idx++;
             }
-            return $newRearranged;
+            $newRearranged[$idx] = $item;
         }
 
-        return $new;
+        ksort($newRearranged);
+        $newRearranged = array_values($newRearranged);
+        return $newRearranged;
     }
 }
diff --git a/src/JsonPatch.php b/src/JsonPatch.php
@@ -92,10 +92,11 @@ public static function import(array $data)
             }
             $op->path = $operation->path;
             if ($op instanceof OpPathValue) {
-                if (!array_key_exists('value', (array)$operation)) {
+                if (property_exists($operation, 'value')) {
+                    $op->value = $operation->value;
+                } else {
                     throw new Exception('Missing "value" in operation data');
                 }
-                $op->value = $operation->value;
             } elseif ($op instanceof OpPathFrom) {
                 if (!isset($operation->from)) {
                     throw new Exception('Missing "from" in operation data');
diff --git a/src/JsonPointer.php b/src/JsonPointer.php
@@ -136,16 +136,16 @@ public static function add(&$holder, $pathItems, $value, $flags = self::RECURSIV
                     if ('-' === $key) {
                         $ref = &$ref[];
                     } else {
-                        if (is_array($ref) && array_key_exists($key, $ref) && empty($pathItems)) {
-                            array_splice($ref, $key, 0, array($value));
-                        }
                         if (false === $intKey) {
                             if (0 === ($flags & self::TOLERATE_ASSOCIATIVE_ARRAYS)) {
                                 throw new Exception('Invalid key for array operation');
                             }
                             $ref = &$ref[$key];
                             continue;
                         }
+                        if (is_array($ref) && array_key_exists($key, $ref) && empty($pathItems)) {
+                            array_splice($ref, $intKey, 0, array($value));
+                        }
                         if (0 === ($flags & self::TOLERATE_ASSOCIATIVE_ARRAYS)) {
                             if ($intKey > count($ref) && 0 === ($flags & self::RECURSIVE_KEY_CREATION)) {
                                 throw new Exception('Index is greater than number of items in array');
diff --git a/src/JsonValueReplace.php b/src/JsonValueReplace.php
@@ -48,6 +48,7 @@ public function process($data)
             }
         }
 
+        /** @var string[] $originalKeys */
         $originalKeys = $data instanceof \stdClass ? get_object_vars($data) : $data;
 
         if ($check) {
diff --git a/tests/src/RearrangeArrayTest.php b/tests/src/RearrangeArrayTest.php

Original file line number	Diff line number	Diff line change
`@@ -92,10 +92,11 @@ public static function import(array $data)`
`92`	`92`	`}`
`93`	`93`	`$op->path = $operation->path;`
`94`	`94`	`if ($op instanceof OpPathValue) {`
`95`		`- if (!array_key_exists('value', (array)$operation)) {`
	`95`	`+ if (property_exists($operation, 'value')) {`
	`96`	`+ $op->value = $operation->value;`
	`97`	`+ } else {`
`96`	`98`	`throw new Exception('Missing "value" in operation data');`
`97`	`99`	`}`
`98`		`- $op->value = $operation->value;`
`99`	`100`	`} elseif ($op instanceof OpPathFrom) {`
`100`	`101`	`if (!isset($operation->from)) {`
`101`	`102`	`throw new Exception('Missing "from" in operation data');`
Original file line number	Diff line number	Diff line change
`@@ -48,6 +48,7 @@ public function process($data)`
`48`	`48`	`}`
`49`	`49`	`}`
`50`	`50`
	`51`	`+ /** @var string[] $originalKeys */`
`51`	`52`	`$originalKeys = $data instanceof \stdClass ? get_object_vars($data) : $data;`
`52`	`53`
`53`	`54`	`if ($check) {`