fixes based on phan results

dealnews · Feb 27, 2024 · a36f896 · a36f896
1 parent 0feec0e
commit a36f896
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 28 deletions.
diff --git a/src/AbstractMapper.php b/src/AbstractMapper.php
@@ -294,6 +294,7 @@ protected function loadRelatedObjects(object $object, string $property, array $m
         $mapper  = new $mapping['mapper']();
         $objects = $mapper->find(
             [
+                // @phan-suppress-next-line PhanTypeArraySuspicious, PhanTypeInvalidDimOffset
                 $mapping['foreign_column'] => $this->getValue($object, $this::PRIMARY_KEY, $this::MAPPING[$this::PRIMARY_KEY]),
             ]
         );
@@ -324,6 +325,7 @@ protected function loadLookupObjects(object $object, string $property, array $ma
         $rows    = $this->crud->read(
             $mapping['table'],
             [
+                // @phan-suppress-next-line PhanTypeArraySuspicious, PhanTypeInvalidDimOffset
                 $mapping['foreign_column'] => $this->getValue($object, $this::PRIMARY_KEY, $this::MAPPING[$this::PRIMARY_KEY]),
             ]
         );
@@ -362,6 +364,7 @@ protected function loadColumnMapper(object $object, string $property, array $map
             $this->crud
         );
 
+        // @phan-suppress-next-line PhanTypeArraySuspicious, PhanTypeInvalidDimOffset
         $data = $mapper->load($this->getValue($object, $this::PRIMARY_KEY, $this::MAPPING[$this::PRIMARY_KEY]));
 
         $this->setValue(
@@ -528,6 +531,7 @@ protected function saveColumnMapper(object $object, string $property, array $map
         $data = $this->getValue($object, $property, $mapping);
 
         $data = $mapper->save(
+            // @phan-suppress-next-line PhanTypeArraySuspicious, PhanTypeInvalidDimOffset
             $this->getValue($object, $this::PRIMARY_KEY, $this::MAPPING[$this::PRIMARY_KEY]),
             $data
         );

diff --git a/src/ColumnMapper.php b/src/ColumnMapper.php
@@ -17,37 +17,49 @@ class ColumnMapper {
      */
     protected CRUD $crud;
 
+    /**
+     * The table with the values
+     */
     protected string $table = '';
+
+    /**
+     * The primary key for the table
+     */
     protected string $primary_key = '';
+
+    /**
+     * The column that has the foreign object pk in it
+     */
     protected string $foreign_column = '';
+
+    /**
+     * The column we want to return in the array
+     */
     protected string $column = '';
 
 
     /**
-     * Creates a new mapper
+     * Constructs a new instance.
      *
-     * @param \DealNews\DB\CRUD|null $crud Optional CRUD object
+     * @param      string  $table           The table
+     * @param      string  $primary_key     The primary key
+     * @param      string  $foreign_column  The foreign column
+     * @param      string  $column          The column
+     * @param      CRUD    $crud            The crud
      */
     public function __construct(
         string $table,
         string $primary_key,
         string $foreign_column,
         string $column,
-        CRUD $crud = null
+        CRUD $crud
     ) {
 
         $this->table          = $table;
         $this->primary_key    = $primary_key;
         $this->foreign_column = $foreign_column;
         $this->column         = $column;
-
-        if ($crud !== null) {
-            $this->crud = $crud;
-        } elseif (!empty($this::DATABASE_NAME)) {
-            $this->crud = new CRUD(\DealNews\DB\Factory::init($this::DATABASE_NAME));
-        } else {
-            throw new \LogicException('No database configuration for ' . get_class($this));
-        }
+        $this->crud           = $crud;
     }
 
     /**
@@ -63,7 +75,7 @@ public function load(int|string $id): array {
         $rows = $this->crud->read(
             $this->table,
             [
-                $this->foreign_column => $id
+                $this->foreign_column => $id,
             ],
             order: $this->column
         );
@@ -84,6 +96,8 @@ public function load(int|string $id): array {
      * @return array
      */
     public function save(int|string $id, array $data): array {
+        $success = true;
+
         $already_in_transaction = $this->crud->pdo->inTransaction();
 
         if (!$already_in_transaction) {
@@ -95,41 +109,47 @@ public function save(int|string $id, array $data): array {
             $existing = $this->crud->read(
                 $this->table,
                 [
-                    $this->foreign_column => $id
+                    $this->foreign_column => $id,
                 ]
             );
 
             foreach ($data as $dk => $value) {
                 foreach ($existing as $ek => $row) {
                     if ($row[$this->column] == $value) {
-                        unset($data[$dk]);
-                        unset($existing[$ek]);
+                        unset($data[$dk], $existing[$ek]);
+
                         break;
                     }
                 }
             }
 
             foreach ($data as $value) {
-                $this->crud->create(
+                $success = $this->crud->create(
                     $this->table,
                     [
                         $this->column         => $value,
                         $this->foreign_column => $id,
                     ]
                 );
+                if (!$success) {
+                    break;
+                }
             }
 
-            foreach ($existing as $ex) {
-                $this->crud->delete(
-                    $this->table,
-                    [
-                        $this->primary_key => $ex[$this->primary_key],
-                    ]
-                );
+            if ($success) {
+                foreach ($existing as $ex) {
+                    $success = $this->crud->delete(
+                        $this->table,
+                        [
+                            $this->primary_key => $ex[$this->primary_key],
+                        ]
+                    );
+                    if (!$success) {
+                        break;
+                    }
+                }
             }
 
-            $data = $this->load($id);
-
         } catch (\PDOException $e) {
             if (!$already_in_transaction) {
                 $this->crud->pdo->rollBack();
@@ -145,6 +165,8 @@ public function save(int|string $id, array $data): array {
             }
         }
 
+        $data = $this->load($id);
+
         return $data;
     }
 }
diff --git a/tests/AbstractMapperTest.php b/tests/AbstractMapperTest.php
@@ -176,9 +176,9 @@ public function testRelationModification() {
 
         $student->courses[0]->name = 'Course 1a';
 
-        unset($student->courses[1]);
+        unset($student->courses[1], $student->nicknames[1]);
+
 
-        unset($student->nicknames[1]);
 
         $mapper  = new StudentMapper();
         $mapper->save($student);

diff --git a/tests/TestClasses/Mapper/StudentMapper.php b/tests/TestClasses/Mapper/StudentMapper.php
@@ -55,7 +55,7 @@ class StudentMapper extends \DealNews\DB\AbstractMapper {
             'table'          => 'student_nicknames',
             'primary_key'    => 'student_nickname_id',
             'foreign_column' => 'student_id',
-            'column'         => 'name'
+            'column'         => 'name',
         ],
     ];
 }
diff --git a/tests/bootstrap.php b/tests/bootstrap.php
@@ -15,7 +15,14 @@
  * does not allow var_dump to send output to STDOUT.
  */
 function _debug() {
+    $bt = debug_backtrace();
     fwrite(STDERR, "\nSTART DEBUG\n");
+    foreach ($bt as $pos => $t) {
+        if (!isset($t['file'])) {
+            break;
+        }
+        fwrite(STDERR, "#$pos {$t['file']} on line {$t['line']}\n");
+    }
     fwrite(STDERR, "###########\n");
     $args = func_get_args();
     foreach ($args as $arg) {