Add protection to physical delete multiplied files, which are related…

… more then one owner
itstructure · Sep 12, 2020 · 08d57c7 · 08d57c7
1 parent ea324c0
commit 08d57c7
Show file tree

Hide file tree

Showing 10 changed files with 84 additions and 60 deletions.
diff --git a/README.md b/README.md
@@ -44,7 +44,7 @@ Addition module description you can see in my [Personal site](https://pack-devel
 
 Via composer:
 
-`composer require itstructure/yii2-multi-format-uploader ~3.1.1`
+`composer require itstructure/yii2-multi-format-uploader ~3.2.0`
 
 ### If you are testing this package from local server directory
 

diff --git a/changelog.md b/changelog.md
@@ -1,5 +1,9 @@
 ### CHANGE LOG:
 
+**3.2.0 September 12, 2020:**
+- Optimize `deleteMediafiles()` method in `MediaFilesTrait`. Add protection to physical delete multiplied files, which are related more then one owner.
+- Optimize owner's entity classes.
+
 **3.1.1 August 10, 2020:**
 - Add module attribute `useInitialThumbsConfig` with default value **true**.
 

diff --git a/src/behaviors/BehaviorAlbum.php b/src/behaviors/BehaviorAlbum.php
@@ -3,6 +3,7 @@
 namespace Itstructure\MFUploader\behaviors;
 
 use yii\db\ActiveRecordInterface;
+use Itstructure\MFUploader\models\OwnerAlbum;
 use Itstructure\MFUploader\models\album\Album;
 
 /**
@@ -38,6 +39,6 @@ protected function loadModel(array $conditions)
      */
     protected function removeOwner(int $ownerId, string $owner, string $ownerAttribute): bool
     {
-        return Album::removeOwner($ownerId, $owner, $ownerAttribute);
+        return OwnerAlbum::removeOwner($ownerId, $owner, $ownerAttribute);
     }
 }
diff --git a/src/behaviors/BehaviorMediafile.php b/src/behaviors/BehaviorMediafile.php
@@ -3,7 +3,7 @@
 namespace Itstructure\MFUploader\behaviors;
 
 use yii\db\ActiveRecordInterface;
-use Itstructure\MFUploader\models\Mediafile;
+use Itstructure\MFUploader\models\{Mediafile, OwnerMediafile};
 
 /**
  * Class BehaviorMediafile
@@ -38,6 +38,6 @@ protected function loadModel(array $conditions)
      */
     protected function removeOwner(int $ownerId, string $owner, string $ownerAttribute): bool
     {
-        return Mediafile::removeOwner($ownerId, $owner, $ownerAttribute);
+        return OwnerMediafile::removeOwner($ownerId, $owner, $ownerAttribute);
     }
 }
diff --git a/src/models/Mediafile.php b/src/models/Mediafile.php
@@ -183,20 +183,6 @@ public function addOwner(int $ownerId, string $owner, string $ownerAttribute): b
         return OwnerMediafile::addOwner($this->id, $ownerId, $owner, $ownerAttribute);
     }
 
-    /**
-     * Remove this mediafile owner.
-     *
-     * @param int    $ownerId
-     * @param string $owner
-     * @param string $ownerAttribute
-     *
-     * @return bool
-     */
-    public static function removeOwner(int $ownerId, string $owner, string $ownerAttribute): bool
-    {
-        return OwnerMediafile::removeOwner($ownerId, $owner, $ownerAttribute);
-    }
-
     /**
      * @return \yii\db\ActiveQuery
      */

diff --git a/src/models/Owner.php b/src/models/Owner.php
@@ -92,21 +92,39 @@ public static function addOwner(int $modelId, int $ownerId, string $owner, strin
      *
      * @param int $ownerId
      * @param string $owner
-     * @param string $ownerAttribute
+     * @param string|null $ownerAttribute
      *
      * @return bool
      */
-    public static function removeOwner(int $ownerId, string $owner, string $ownerAttribute): bool
+    public static function removeOwner(int $ownerId, string $owner, string $ownerAttribute = null): bool
     {
-        $deleted = static::deleteAll([
-            'ownerId' => $ownerId,
-            'owner' => $owner,
-            'ownerAttribute' => $ownerAttribute,
-        ]);
+        $deleted = static::deleteAll(static::buildFilterOptions($ownerId, $owner, $ownerAttribute));
 
         return $deleted > 0;
     }
 
+    /**
+     * Getting entity id's which are related with Other owners too.
+     *
+     * @param string $owner
+     * @param int $ownerId
+     * @param array $entityIds
+     *
+     * @return array|\yii\db\ActiveRecord[]
+     */
+    public static function filterMultipliedEntityIds(string $owner, int $ownerId, array $entityIds)
+    {
+        return static::find()
+            ->select(static::getModelKeyName())
+            ->where([static::getModelKeyName() => $entityIds])
+            ->andWhere([
+                'OR',
+                ['!=', 'ownerId', $ownerId],
+                ['!=', 'owner', $owner]
+            ])
+            ->all();
+    }
+
     /**
      * Get Id's by owner.
      *
@@ -121,22 +139,22 @@ protected static function getEntityIdsQuery(string $nameId, array $args): Active
     {
         $conditions = [];
 
-        if (isset($args['owner'])) {
-            if (!is_string($args['owner']) || empty($args['owner'])) {
+        if (!empty($args['owner'])) {
+            if (!is_string($args['owner'])) {
                 throw new InvalidArgumentException('Parameter owner must be a string.');
             }
             $conditions['owner'] = $args['owner'];
 
-            if (isset($args['ownerId'])) {
+            if (!empty($args['ownerId'])) {
                 if (!is_numeric($args['ownerId'])) {
                     throw new InvalidArgumentException('Parameter ownerId must be numeric.');
                 }
                 $conditions['ownerId'] = $args['ownerId'];
             }
         }
 
-        if (isset($args['ownerAttribute'])) {
-            if (!is_string($args['ownerAttribute']) || empty($args['ownerAttribute'])) {
+        if (!empty($args['ownerAttribute'])) {
+            if (!is_string($args['ownerAttribute'])) {
                 throw new InvalidArgumentException('Parameter ownerAttribute must be a string.');
             }
             $conditions['ownerAttribute'] = $args['ownerAttribute'];
@@ -151,4 +169,21 @@ protected static function getEntityIdsQuery(string $nameId, array $args): Active
 
         return $query;
     }
+
+    /**
+     * Build filter options for some actions.
+     *
+     * @param int $ownerId
+     * @param string $owner
+     * @param string|null $ownerAttribute
+     *
+     * @return array
+     */
+    protected static function buildFilterOptions(int $ownerId, string $owner, string $ownerAttribute = null)
+    {
+        return array_merge([
+            'ownerId' => $ownerId,
+            'owner' => $owner
+        ], empty($ownerAttribute) ? [] : ['ownerAttribute' => $ownerAttribute]);
+    }
 }
diff --git a/src/models/OwnerAlbum.php b/src/models/OwnerAlbum.php
@@ -95,11 +95,7 @@ public function getAlbum()
      */
     public static function getAlbums(string $owner, int $ownerId, string $ownerAttribute = null)
     {
-        return static::getAlbumsQuery([
-            'owner' => $owner,
-            'ownerId' => $ownerId,
-            'ownerAttribute' => $ownerAttribute,
-        ])->all();
+        return static::getAlbumsQuery(static::buildFilterOptions($ownerId, $owner, $ownerAttribute))->all();
     }
 
     /**

diff --git a/src/models/OwnerMediafile.php b/src/models/OwnerMediafile.php
@@ -95,11 +95,7 @@ public function getMediaFile()
      */
     public static function getMediaFiles(string $owner, int $ownerId, string $ownerAttribute = null)
     {
-        return static::getMediaFilesQuery([
-            'owner' => $owner,
-            'ownerId' => $ownerId,
-            'ownerAttribute' => $ownerAttribute,
-        ])->all();
+        return static::getMediaFilesQuery(static::buildFilterOptions($ownerId, $owner, $ownerAttribute))->all();
     }
 
     /**

diff --git a/src/models/album/Album.php b/src/models/album/Album.php
@@ -187,20 +187,6 @@ public function addOwner(int $ownerId, string $owner, string $ownerAttribute): b
         return OwnerAlbum::addOwner($this->id, $ownerId, $owner, $ownerAttribute);
     }
 
-    /**
-     * Remove this mediafile owner.
-     *
-     * @param int    $ownerId
-     * @param string $owner
-     * @param string $ownerAttribute
-     *
-     * @return bool
-     */
-    public static function removeOwner(int $ownerId, string $owner, string $ownerAttribute): bool
-    {
-        return OwnerAlbum::removeOwner($ownerId, $owner, $ownerAttribute);
-    }
-
     /**
      * Get album's owners.
      *

diff --git a/src/traits/MediaFilesTrait.php b/src/traits/MediaFilesTrait.php
@@ -35,19 +35,39 @@ trait MediaFilesTrait
      * @param string $owner
      * @param int $ownerId
      * @param Module $module
+     * @param bool $protectMultiplied
      *
      * @return void
      */
-    protected function deleteMediafiles(string $owner, int $ownerId, Module $module): void
+    protected function deleteMediafiles(string $owner, int $ownerId, Module $module, bool $protectMultiplied = true): void
     {
-        $mediafileIds = OwnerMediafile::getMediaFilesQuery([
+        $mediafileIds = array_map(function ($data) {
+            return $data->id;
+
+        }, OwnerMediafile::getMediaFilesQuery([
             'owner' => $owner,
-            'ownerId' => $ownerId,
+            'ownerId' => $ownerId
         ])
-        ->select('id')
-        ->all();
+            ->select('id')
+            ->all()
+        );
+
+        if ($protectMultiplied) {
+            $multipliedMediafileIds = array_map(function ($data) {
+                return $data->mediafileId;
+
+            }, OwnerMediafile::filterMultipliedEntityIds($owner, $ownerId, $mediafileIds));
 
-        $mediafileIds = array_map(function ($data) {return $data->id;}, $mediafileIds);
+            $mediafileIds = array_filter($mediafileIds, function ($item) use ($multipliedMediafileIds) {
+                return !in_array($item, $multipliedMediafileIds);
+            });
+
+            OwnerMediafile::deleteAll([
+                'owner' => $owner,
+                'ownerId' => $ownerId,
+                'mediafileId' => $multipliedMediafileIds
+            ]);
+        }
 
         $this->deleteMediafileEntry($mediafileIds, $module);
     }