Merge pull request #6060 from pixelfed/staging

Update ImportService and TransformImports to fix race condition bug

app/Console/Commands/TransformImports.php

@@ -10,10 +10,10 @@ use App\Services\ImportService;
 use App\Services\MediaPathService;
 use App\Status;
 use Illuminate\Console\Command;
-use Illuminate\Database\QueryException;
-use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Str;
 use Storage;
+use Illuminate\Database\QueryException;
+use Illuminate\Support\Facades\DB;
 
 class TransformImports extends Command
 {
@@ -76,56 +76,9 @@ class TransformImports extends Command
             if (! $idk) {
                 $ip->skip_missing_media = true;
                 $ip->save();
-
-                continue;
-            }
-
-            $originalIncr = $idk['incr'];
-            $attempts = 0;
-            while ($attempts < 999) {
-                $duplicateCheck = ImportPost::where('user_id', $id)
-                    ->where('creation_year', $ip->creation_year)
-                    ->where('creation_month', $ip->creation_month)
-                    ->where('creation_day', $ip->creation_day)
-                    ->where('creation_id', $idk['incr'])
-                    ->where('id', '!=', $ip->id)
-                    ->exists();
-
-                if (! $duplicateCheck) {
-                    break;
-                }
-
-                $idk['incr']++;
-                $attempts++;
-
-                if ($idk['incr'] > 999) {
-                    $this->warn("Could not find unique creation_id for ImportPost ID {$ip->id} on {$ip->creation_year}-{$ip->creation_month}-{$ip->creation_day}");
-                    $ip->skip_missing_media = true;
-                    $ip->save();
-
-                    continue 2;
-                }
-            }
-
-            if ($attempts >= 999) {
-                $this->warn("Exhausted attempts finding unique creation_id for ImportPost ID {$ip->id}");
-                $ip->skip_missing_media = true;
-                $ip->save();
-
                 continue;
             }
 
-            if ($idk['incr'] !== $originalIncr) {
-                $uid = str_pad($id, 6, 0, STR_PAD_LEFT);
-                $yearStr = str_pad($ip->creation_year, 2, 0, STR_PAD_LEFT);
-                $monthStr = str_pad($ip->creation_month, 2, 0, STR_PAD_LEFT);
-                $dayStr = str_pad($ip->creation_day, 2, 0, STR_PAD_LEFT);
-                $zone = $yearStr.$monthStr.$dayStr.str_pad($idk['incr'], 3, 0, STR_PAD_LEFT);
-                $idk['id'] = '1'.$uid.$zone;
-
-                $this->info("Adjusted creation_id from {$originalIncr} to {$idk['incr']} for ImportPost ID {$ip->id}");
-            }
-
             if (Storage::exists('imports/'.$id.'/'.$ip->filename) === false) {
                 ImportService::clearAttempts($profile->id);
                 ImportService::getPostCount($profile->id, true);
@@ -151,7 +104,7 @@ class TransformImports extends Command
                 continue;
             }
 
-            $caption = $ip->caption ?? '';
+            $caption = $ip->caption ?? "";
             $status = new Status;
             $status->profile_id = $pid;
             $status->caption = $caption;
@@ -190,9 +143,33 @@ class TransformImports extends Command
             }
 
             try {
-                DB::transaction(function () use ($ip, $status, $profile, $idk) {
+                DB::transaction(function () use ($ip, $status, $profile, $id) {
+                    $uniqueIdData = ImportService::getUniqueCreationId(
+                        $id,
+                        $ip->creation_year,
+                        $ip->creation_month,
+                        $ip->creation_day,
+                        $ip->id
+                    );
+
+                    if (!$uniqueIdData) {
+                        throw new \Exception("Could not generate unique creation_id for ImportPost ID {$ip->id}");
+                    }
+
                     $ip->status_id = $status->id;
-                    $ip->creation_id = $idk['incr'];
+                    $ip->creation_id = $uniqueIdData['incr'];
+
+                    if ($uniqueIdData['year'] !== $ip->creation_year ||
+                        $uniqueIdData['month'] !== $ip->creation_month ||
+                        $uniqueIdData['day'] !== $ip->creation_day) {
+
+                        $ip->creation_year = $uniqueIdData['year'];
+                        $ip->creation_month = $uniqueIdData['month'];
+                        $ip->creation_day = $uniqueIdData['day'];
+
+                        $this->info("Date shifted for ImportPost ID {$ip->id} to {$uniqueIdData['year']}-{$uniqueIdData['month']}-{$uniqueIdData['day']}");
+                    }
+
                     $ip->save();
 
                     $profile->status_count = $profile->status_count + 1;
@@ -204,18 +181,23 @@ class TransformImports extends Command
                 ImportService::getPostCount($profile->id, true);
 
             } catch (QueryException $e) {
-                if ($e->getCode() === '23000') {
-                    $this->warn("Constraint violation for ImportPost ID {$ip->id}: ".$e->getMessage());
-                    $ip->skip_missing_media = true;
-                    $ip->save();
+                $this->error("Database error for ImportPost ID {$ip->id}: " . $e->getMessage());
+                $ip->skip_missing_media = true;
+                $ip->save();
 
-                    Media::where('status_id', $status->id)->delete();
-                    $status->delete();
+                $status->delete();
+                Media::where('status_id', $status->id)->delete();
 
-                    continue;
-                } else {
-                    throw $e;
-                }
+                continue;
+            } catch (\Exception $e) {
+                $this->error("Error processing ImportPost ID {$ip->id}: " . $e->getMessage());
+                $ip->skip_missing_media = true;
+                $ip->save();
+
+                $status->delete();
+                Media::where('status_id', $status->id)->delete();
+
+                continue;
             }
         }
     }

app/Services/ImportService.php

@@ -4,6 +4,8 @@ namespace App\Services;
 
 use App\Models\ImportPost;
 use Cache;
+use DB;
+use Illuminate\Database\QueryException;
 
 class ImportService
 {
@@ -11,71 +13,227 @@ class ImportService
 
     public static function getId($userId, $year, $month, $day)
     {
-        if($userId > 999999) {
-            return;
+        if ($userId > 999999) {
+            return null;
         }
-        if($year < 9 || $year > (int) now()->addYear()->format('y')) {
-            return;
+        if ($year < 9 || $year > (int) now()->addYear()->format('y')) {
+            return null;
         }
-        if($month < 1 || $month > 12) {
-            return;
+        if ($month < 1 || $month > 12) {
+            return null;
         }
-        if($day < 1 || $day > 31) {
-            return;
+        if ($day < 1 || $day > 31) {
+            return null;
         }
+
         $start = 1;
-        $key = self::CACHE_KEY . 'getIdRange:incr:byUserId:' . $userId . ':y-' . $year . ':m-' . $month . ':d-' . $day;
-        $incr = Cache::increment($key, random_int(5, 19));
-        if($incr > 999) {
-            $daysInMonth = now()->parse($day . '-' . $month . '-' . $year)->daysInMonth;
-
-            if($month == 12) {
-                $year = $year + 1;
-                $month = 1;
-                $day = 0;
+        $maxAttempts = 10;
+
+        for ($attempt = 0; $attempt < $maxAttempts; $attempt++) {
+            try {
+                return DB::transaction(function () use ($userId, $year, $month, $day, $start) {
+                    $maxExistingIncr = ImportPost::where('user_id', $userId)
+                        ->where('creation_year', $year)
+                        ->where('creation_month', $month)
+                        ->where('creation_day', $day)
+                        ->lockForUpdate()
+                        ->max('creation_id') ?? 0;
+
+                    $incr = $maxExistingIncr + 1;
+
+                    // If we've exceeded 999 for this day, try next day
+                    if ($incr > 999) {
+                        [$newYear, $newMonth, $newDay] = self::getNextValidDate($year, $month, $day);
+                        if (! $newYear) {
+                            throw new \Exception('Could not find valid next date');
+                        }
+
+                        return self::getId($userId, $newYear, $newMonth, $newDay);
+                    }
+
+                    $uid = str_pad($userId, 6, 0, STR_PAD_LEFT);
+                    $yearStr = str_pad($year, 2, 0, STR_PAD_LEFT);
+                    $monthStr = str_pad($month, 2, 0, STR_PAD_LEFT);
+                    $dayStr = str_pad($day, 2, 0, STR_PAD_LEFT);
+                    $zone = $yearStr.$monthStr.$dayStr.str_pad($incr, 3, 0, STR_PAD_LEFT);
+
+                    return [
+                        'id' => $start.$uid.$zone,
+                        'year' => $year,
+                        'month' => $month,
+                        'day' => $day,
+                        'incr' => $incr,
+                        'user_id' => $userId,
+                    ];
+                }, 3);
+            } catch (QueryException $e) {
+                if ($e->getCode() === '40001') {
+                    usleep(random_int(1000, 10000));
+
+                    continue;
+                }
+                throw $e;
+            } catch (\Exception $e) {
+                if (strpos($e->getMessage(), 'Could not find valid next date') !== false) {
+                    return null;
+                }
+                throw $e;
             }
+        }
+
+        return null;
+    }
+
+    public static function getAndReserveId($userId, $year, $month, $day, $importPostData = [])
+    {
+        if ($userId > 999999) {
+            return null;
+        }
+        if ($year < 9 || $year > (int) now()->addYear()->format('y')) {
+            return null;
+        }
+        if ($month < 1 || $month > 12) {
+            return null;
+        }
+        if ($day < 1 || $day > 31) {
+            return null;
+        }
+
+        $start = 1;
+        $maxAttempts = 10;
+
+        for ($attempt = 0; $attempt < $maxAttempts; $attempt++) {
+            try {
+                return DB::transaction(function () use ($userId, $year, $month, $day, $start, $importPostData) {
+                    $maxExistingIncr = ImportPost::where('user_id', $userId)
+                        ->where('creation_year', $year)
+                        ->where('creation_month', $month)
+                        ->where('creation_day', $day)
+                        ->lockForUpdate()
+                        ->max('creation_id') ?? 0;
+
+                    $incr = $maxExistingIncr + 1;
+
+                    if ($incr > 999) {
+                        [$newYear, $newMonth, $newDay] = self::getNextValidDate($year, $month, $day);
+                        if (! $newYear) {
+                            throw new \Exception('Could not find valid next date');
+                        }
+
+                        return self::getAndReserveId($userId, $newYear, $newMonth, $newDay, $importPostData);
+                    }
+
+                    $uid = str_pad($userId, 6, 0, STR_PAD_LEFT);
+                    $yearStr = str_pad($year, 2, 0, STR_PAD_LEFT);
+                    $monthStr = str_pad($month, 2, 0, STR_PAD_LEFT);
+                    $dayStr = str_pad($day, 2, 0, STR_PAD_LEFT);
+                    $zone = $yearStr.$monthStr.$dayStr.str_pad($incr, 3, 0, STR_PAD_LEFT);
+
+                    $idData = [
+                        'id' => $start.$uid.$zone,
+                        'year' => $year,
+                        'month' => $month,
+                        'day' => $day,
+                        'incr' => $incr,
+                        'user_id' => $userId,
+                    ];
+
+                    $placeholder = new ImportPost(array_merge([
+                        'user_id' => $userId,
+                        'creation_year' => $year,
+                        'creation_month' => $month,
+                        'creation_day' => $day,
+                        'creation_id' => $incr,
+                        'reserved_at' => now(),
+                    ], $importPostData));
+
+                    $placeholder->save();
 
-            if($day + 1 >= $daysInMonth) {
-                $day = 1;
-                $month = $month + 1;
-            } else {
-                $day = $day + 1;
+                    return [
+                        'id_data' => $idData,
+                        'import_post' => $placeholder,
+                    ];
+                }, 3);
+            } catch (QueryException $e) {
+                if ($e->getCode() === '40001') {
+                    usleep(random_int(1000, 10000));
+
+                    continue;
+                }
+
+                if ($e->getCode() === '23000') {
+                    usleep(random_int(100, 1000));
+
+                    continue;
+                }
+
+                throw $e;
+            } catch (\Exception $e) {
+                if (strpos($e->getMessage(), 'Could not find valid next date') !== false) {
+                    return null;
+                }
+                throw $e;
             }
-            return self::getId($userId, $year, $month, $day);
         }
-        $uid = str_pad($userId, 6, 0, STR_PAD_LEFT);
-        $year = str_pad($year, 2, 0, STR_PAD_LEFT);
-        $month = str_pad($month, 2, 0, STR_PAD_LEFT);
-        $day = str_pad($day, 2, 0, STR_PAD_LEFT);
-        $zone = $year . $month . $day . str_pad($incr, 3, 0, STR_PAD_LEFT);
-        return [
-            'id' => $start . $uid . $zone,
-            'year' => $year,
-            'month' => $month,
-            'day' => $day,
-            'incr' => $incr,
-        ];
+
+        return null;
+    }
+
+    public static function getUniqueCreationId($userId, $year, $month, $day, $excludeImportPostId = null)
+    {
+        return DB::transaction(function () use ($userId, $year, $month, $day, $excludeImportPostId) {
+            $query = ImportPost::where('user_id', $userId)
+                ->where('creation_year', $year)
+                ->where('creation_month', $month)
+                ->where('creation_day', $day)
+                ->lockForUpdate();
+
+            if ($excludeImportPostId) {
+                $query->where('id', '!=', $excludeImportPostId);
+            }
+
+            $maxExistingIncr = $query->max('creation_id') ?? 0;
+            $incr = $maxExistingIncr + 1;
+
+            if ($incr > 999) {
+                [$newYear, $newMonth, $newDay] = self::getNextValidDate($year, $month, $day);
+                if (! $newYear) {
+                    return null;
+                }
+
+                return self::getUniqueCreationId($userId, $newYear, $newMonth, $newDay, $excludeImportPostId);
+            }
+
+            return [
+                'incr' => $incr,
+                'year' => $year,
+                'month' => $month,
+                'day' => $day,
+            ];
+        }, 3);
     }
 
     public static function getPostCount($profileId, $refresh = false)
     {
-        $key = self::CACHE_KEY . 'totalPostCountByProfileId:' . $profileId;
-        if($refresh) {
+        $key = self::CACHE_KEY.'totalPostCountByProfileId:'.$profileId;
+        if ($refresh) {
             Cache::forget($key);
         }
-        return intval(Cache::remember($key, 21600, function() use($profileId) {
+
+        return intval(Cache::remember($key, 21600, function () use ($profileId) {
             return ImportPost::whereProfileId($profileId)->whereSkipMissingMedia(false)->count();
         }));
     }
 
     public static function getAttempts($profileId)
     {
-        $key = self::CACHE_KEY . 'attemptsByProfileId:' . $profileId;
-        return intval(Cache::remember($key, 21600, function() use($profileId) {
+        $key = self::CACHE_KEY.'attemptsByProfileId:'.$profileId;
+
+        return intval(Cache::remember($key, 21600, function () use ($profileId) {
             return ImportPost::whereProfileId($profileId)
                 ->whereSkipMissingMedia(false)
                 ->get()
-                ->groupBy(function($item) {
+                ->groupBy(function ($item) {
                     return $item->created_at->format('Y-m-d');
                 })
                 ->count();
@@ -84,31 +242,55 @@ class ImportService
 
     public static function clearAttempts($profileId)
     {
-        $key = self::CACHE_KEY . 'attemptsByProfileId:' . $profileId;
+        $key = self::CACHE_KEY.'attemptsByProfileId:'.$profileId;
+
         return Cache::forget($key);
     }
 
     public static function getImportedFiles($profileId, $refresh = false)
     {
-        $key = self::CACHE_KEY . 'importedPostsByProfileId:' . $profileId;
-        if($refresh) {
+        $key = self::CACHE_KEY.'importedPostsByProfileId:'.$profileId;
+        if ($refresh) {
             Cache::forget($key);
         }
-        return Cache::remember($key, 21600, function() use($profileId) {
+
+        return Cache::remember($key, 21600, function () use ($profileId) {
             return ImportPost::whereProfileId($profileId)
                 ->get()
-                ->filter(function($ip) {
+                ->filter(function ($ip) {
                     return StatusService::get($ip->status_id) == null;
                 })
-                ->map(function($ip) {
-                    return collect($ip->media)->map(function($m) { return $m['uri']; });
+                ->map(function ($ip) {
+                    return collect($ip->media)->map(function ($m) {
+                        return $m['uri'];
+                    });
                 })->values()->flatten();
         });
     }
 
     public static function clearImportedFiles($profileId)
     {
-        $key = self::CACHE_KEY . 'importedPostsByProfileId:' . $profileId;
+        $key = self::CACHE_KEY.'importedPostsByProfileId:'.$profileId;
+
         return Cache::forget($key);
     }
+
+    private static function getNextValidDate($year, $month, $day)
+    {
+        try {
+            $fullYear = $year < 50 ? 2000 + $year : 1900 + $year;
+            $date = \Carbon\Carbon::createFromDate($fullYear, $month, $day);
+            $nextDay = $date->addDay();
+
+            $nextYear2Digit = (int) $nextDay->format('y');
+
+            return [
+                $nextYear2Digit,
+                $nextDay->month,
+                $nextDay->day,
+            ];
+        } catch (\Exception $e) {
+            return [null, null, null];
+        }
+    }
 }