Merge remote-tracking branch 'origin/MAGETWO-39769-pk-autoincrement' into MAGETWO-43197-missing-interceptor

Author: Joan He

app/code/Magento/Tax/Model/Resource/Calculation.php

@@ -24,13 +24,6 @@ class Calculation extends \Magento\Framework\Model\Resource\Db\AbstractDb
      */
     protected $_ratesCache = [];
 
-    /**
-     * Primary key auto increment flag
-     *
-     * @var bool
-     */
-    protected $_isPkAutoIncrement = false;
-
     /**
      * Tax data
      *

lib/internal/Magento/Framework/Model/Resource/Db/AbstractDb.php

@@ -768,7 +768,9 @@ protected function saveNewObject(\Magento\Framework\Model\AbstractModel $object)
         }
         $this->getConnection()->insert($this->getMainTable(), $bind);
 
-        $object->setId($this->getConnection()->lastInsertId($this->getMainTable()));
+        if ($this->_isPkAutoIncrement) {
+            $object->setId($this->getConnection()->lastInsertId($this->getMainTable()));
+        }
 
         if ($this->_useIsObjectNew) {
             $object->isObjectNew(false);

lib/internal/Magento/Framework/Model/Test/Unit/Resource/Db/AbstractDbTest.php

@@ -487,4 +487,72 @@ public function testPrepareDataForUpdate()
 
         $this->_model->save($abstractModelMock);
     }
+
+    /**
+     * Test that we only set/override id on object if PK autoincrement is enabled
+     * @param bool $pkIncrement
+     * @dataProvider saveNewObjectDataProvider
+     * @SuppressWarnings(PHPMD.NPathComplexity)
+     */
+    public function testSaveNewObject($pkIncrement)
+    {
+        /**
+         * Mock SUT so as not to test extraneous logic
+         */
+        $model = $this->getMockBuilder('Magento\Framework\Model\Resource\Db\AbstractDb')
+            ->disableOriginalConstructor()
+            ->setMethods(['_prepareDataForSave', 'getIdFieldName', 'getConnection', 'getMainTable'])
+            ->getMockForAbstractClass();
+        /**
+         * Only testing the logic in a protected method and property, must use reflection to avoid dealing with large
+         * amounts of unrelated logic in save function
+         *
+         * make saveNewObject and _isPkAutoIncrement public
+         */
+        $reflectionMethod = new \ReflectionMethod($model, 'saveNewObject');
+        $reflectionMethod->setAccessible(true);
+        $reflectionProperty = new \ReflectionProperty($model, '_isPkAutoIncrement');
+        $reflectionProperty->setAccessible(true);
+        $reflectionProperty->setValue($model, $pkIncrement);
+
+        // Mocked behavior
+        $connectionMock = $this->getMockBuilder('\Magento\Framework\DB\Adapter\AdapterInterface')
+            ->disableOriginalConstructor()
+            ->setMethods(['lastInsertId'])
+            ->getMockForAbstractClass();
+        $getConnectionInvokedCount = $pkIncrement ? 2 : 1;
+        $model->expects($this->exactly($getConnectionInvokedCount))
+            ->method('getConnection')
+            ->willReturn($connectionMock);
+
+        $idFieldName = 'id_field_name';
+        $model->expects($this->once())->method('_prepareDataForSave')->willReturn([$idFieldName => 'id',]);
+
+
+        // Test expectations
+        //      Only get object's id field name if not PK autoincrement
+        $getIdFieldNameInvokedCount = $pkIncrement ? 1 : 0;
+        $model->expects($this->exactly($getIdFieldNameInvokedCount))
+            ->method('getIdFieldName')
+            ->willReturn($idFieldName);
+
+        //      Only set object id if not PK autoincrement
+        $setIdInvokedCount = $pkIncrement ? 1 : 0;
+        $inputObject = $this->getMockBuilder('\Magento\Framework\Model\AbstractModel')
+            ->disableOriginalConstructor()
+            ->getMock();
+        $inputObject->expects($this->exactly($setIdInvokedCount))->method('setId');
+
+        //      Only call lastInsertId if not PK autoincrement
+        $lastInsertIdInvokedCount = $pkIncrement ? 1 : 0;
+        $connectionMock->expects($this->exactly($lastInsertIdInvokedCount))->method('lastInsertId');
+
+        $reflectionMethod->invokeArgs($model, [$inputObject]);
+    }
+
+    public function saveNewObjectDataProvider()
+    {
+        return [[true], [false]];
+    }
+
 }