Parcourir la source

implementing writing order of images on upload.
Fixes order of images beeing sometimes not chronological due to S3 Upload timing variance

Medowar il y a 1 mois
Parent
commit
26576658b9
7 fichiers modifiés avec 148 ajouts et 23 suppressions
  1. 6 1
      admin/api.php
  2. 46 9
      app/s3.php
  3. 50 2
      app/storage.php
  4. 25 6
      assets/admin.js
  5. 13 3
      docs/ARCHITECTURE.md
  6. 3 0
      docs/SETUP.md
  7. 5 2
      upload-api.php

+ 6 - 1
admin/api.php

@@ -12,6 +12,9 @@
  *   slug      gallery slug
  *   original  the full-resolution file (required, stored unmodified)
  *   thumb     browser-generated JPEG thumbnail (optional; absent for RAW/video)
+ *   batch     id of the selection this file came from (optional)
+ *   seq       its position within that selection, so parallel uploads are
+ *             stored in the order they were picked, not the order they land
  *
  * The browser never receives an S3 URL or any credential for writing.
  */
@@ -49,6 +52,8 @@ if ($gallery === null) {
 [$status, $payload] = gallery_store_s3_upload(
     $gallery,
     $_FILES['original'] ?? null,
-    $_FILES['thumb'] ?? null
+    $_FILES['thumb'] ?? null,
+    false,
+    $_POST
 );
 json_response($payload, $status);

+ 46 - 9
app/s3.php

@@ -601,27 +601,63 @@ function upload_error_message(int $code): string
     };
 }
 
+/**
+ * The ordering pair the uploader sends with every file, sanitised for storage:
+ *
+ *   batch  opaque id shared by all files of one drop/selection
+ *   seq    the file's position within that batch
+ *
+ * Returns [] when either is absent or malformed — an upload without usable
+ * ordering is simply appended at the end, which is what every upload did before
+ * this existed, so an older cached admin.js keeps working.
+ *
+ * The batch id is never interpreted, only compared, so the guest-facing endpoint
+ * can accept it from an unauthenticated browser: the worst a crafted value can
+ * do is place the sender's own upload among its own siblings. It is still capped
+ * and stripped to keep the gallery JSON tidy.
+ */
+function upload_order_fields(array $fields): array
+{
+    // is_string, not a cast: a client is free to post batch[]=… as an array.
+    if (!is_string($fields['batch'] ?? null) || !is_numeric($fields['seq'] ?? null)) {
+        return [];
+    }
+    $batch = preg_replace('/[^A-Za-z0-9_-]+/', '', $fields['batch']) ?? '';
+    if ($batch === '') {
+        return [];
+    }
+    return ['batch' => substr($batch, 0, 32), 'seq' => max(0, (int)$fields['seq'])];
+}
+
 /**
  * Ingest one uploaded image into a gallery: stream the original (and optional
- * browser-generated thumbnail) to S3, then append it to the gallery's JSON file.
+ * browser-generated thumbnail) to S3, then store it in the gallery's JSON file.
  *
  * Shared by admin/api.php (trusted admin) and upload-api.php (public guest link).
- * The browser uploads several images at once, so the gallery entry is appended
+ * The browser uploads several images at once, so the gallery entry goes in
  * through gallery_append_image(), which re-reads and rewrites the JSON file
  * under an exclusive lock — two uploads finishing together cannot drop one
- * another's entry. Object keys are generated server-side under the gallery's
- * own prefix — never taken from the client.
+ * another's entry — and places it by the batch/seq the browser sent rather than
+ * at the end, so the gallery keeps the order the files were selected in.
+ * Object keys are generated server-side under the gallery's own prefix — never
+ * taken from the client.
  *
  * $original / $thumb are $_FILES entries (or null). When $imagesOnly is true the
  * original must have a recognised image extension and decode via getimagesize(),
- * so a public link cannot be used to store arbitrary file types.
+ * so a public link cannot be used to store arbitrary file types. $fields is the
+ * request's $_POST, read for the ordering pair only.
  *
  * Returns [int $httpStatus, array $payload] for the caller to hand to
  * json_response(); a thumbnail failure is non-fatal (the grid falls back to the
  * original key).
  */
-function gallery_store_s3_upload(array $gallery, ?array $original, ?array $thumb, bool $imagesOnly = false): array
-{
+function gallery_store_s3_upload(
+    array $gallery,
+    ?array $original,
+    ?array $thumb,
+    bool $imagesOnly = false,
+    array $fields = []
+): array {
     if (!is_array($original) || ($original['error'] ?? UPLOAD_ERR_NO_FILE) !== UPLOAD_ERR_OK) {
         return [400, ['error' => upload_error_message((int)($original['error'] ?? UPLOAD_ERR_NO_FILE))]];
     }
@@ -665,13 +701,14 @@ function gallery_store_s3_upload(array $gallery, ?array $original, ?array $thumb
         }
     }
 
-    // Locked read-modify-write: concurrent uploads append without clobbering.
+    // Locked read-modify-write: concurrent uploads are placed in selection
+    // order (see gallery_image_position) without clobbering each other.
     $count = gallery_append_image($slug, [
         'key'   => $key,
         'thumb' => $thumbKey,
         'name'  => substr((string)($original['name'] ?? basename($key)), 0, 200),
         'size'  => (int)($original['size'] ?? 0),
-    ]);
+    ] + upload_order_fields($fields));
 
     // The gallery was deleted while this image was in flight: drop the objects
     // we just wrote rather than leaving them unreferenced in the bucket.

+ 50 - 2
app/storage.php

@@ -284,9 +284,50 @@ function gallery_delete(string $slug): void
 }
 
 /**
- * Append one image to a gallery under an exclusive lock, so parallel uploads
+ * Where a newly uploaded image belongs among the ones already stored.
+ *
+ * Uploads run several at a time, so they finish in an order set by file size
+ * and network luck, not by the order the photographer picked them. Each job
+ * therefore carries the batch it was selected in and its position within that
+ * batch ($image['batch'] / $image['seq']), and lands next to its siblings
+ * instead of wherever it happened to arrive.
+ *
+ * A batch occupies one contiguous run: its first arrival appends at the end,
+ * and every later one inserts inside that run, which only shifts the runs after
+ * it. So dropping a second selection while the first is still uploading keeps
+ * the two apart, in the order they were dropped.
+ *
+ * Returns the insert position, or null to append — for an unknown batch, and
+ * for images stored before this ordering existed (no batch at all).
+ */
+function gallery_image_position(array $images, array $image): ?int
+{
+    $batch = $image['batch'] ?? null;
+    if (!is_string($batch) || $batch === '') {
+        return null;
+    }
+
+    $seq = (int)($image['seq'] ?? 0);
+    $pos = null;
+    foreach ($images as $i => $existing) {
+        if (($existing['batch'] ?? null) !== $batch) {
+            continue;
+        }
+        if ((int)($existing['seq'] ?? 0) > $seq) {
+            return $i; // first sibling that belongs after us
+        }
+        $pos = $i + 1;
+    }
+    return $pos;
+}
+
+/**
+ * Insert one image into a gallery under an exclusive lock, so parallel uploads
  * into the same gallery cannot overwrite each other's entries.
  *
+ * Position comes from gallery_image_position(), so the stored order follows the
+ * selection order rather than the order the uploads completed in.
+ *
  * Returns the new image count, or null if the gallery no longer exists — an
  * absent gallery must not be resurrected as a stub by a late upload.
  */
@@ -298,7 +339,14 @@ function gallery_append_image(string $slug, array $image): ?int
             $missing = true;
             return null; // deleted mid-upload — do not write a stub file back
         }
-        $g['images'][] = $image;
+        $images = $g['images'] ?? [];
+        $at = gallery_image_position($images, $image);
+        if ($at === null) {
+            $images[] = $image;
+        } else {
+            array_splice($images, $at, 0, [$image]);
+        }
+        $g['images'] = $images;
         return $g;
     });
     if ($missing) {

+ 25 - 6
assets/admin.js

@@ -15,8 +15,10 @@
  * is store-and-forward — the webhost receives the whole body before it starts
  * the S3 PUT — so a single-file queue leaves the uplink idle for the entire
  * webhost→S3 leg and for every thumbnail decode. Overlapping requests keeps it
- * saturated; the server appends to the gallery JSON under a lock, so parallel
- * completions cannot lose entries.
+ * saturated; the server writes the gallery JSON under a lock, so parallel
+ * completions cannot lose entries, and each file carries the batch it was
+ * selected in plus its position there, so they are stored in the order they
+ * were picked rather than the order they happen to finish in.
  */
 (function () {
     'use strict';
@@ -65,14 +67,25 @@
         if (active || queue.length) { e.preventDefault(); e.returnValue = ''; }
     });
 
+    /* One selection (a drop, or one trip through the file dialog) is a batch,
+       and every file remembers its place in it. Uploads finish in an order set
+       by file size and network luck, so without this the gallery would store
+       them shuffled; the server puts each one back among its own siblings.
+       The id is opaque — the server only compares it, never reads it — so a
+       random token is enough and no clock has to be trusted. */
+    function batchId() {
+        return Date.now().toString(36) + Math.random().toString(36).slice(2, 8);
+    }
+
     function enqueue(files) {
-        Array.prototype.forEach.call(files, function (file) {
+        var batch = batchId();
+        Array.prototype.forEach.call(files, function (file, i) {
             var row = document.createElement('div');
             row.className = 'upload-item';
             row.innerHTML = '<span class="name"></span><span class="bar"><i></i></span><span class="state">queued</span>';
             row.querySelector('.name').textContent = file.name;
             list.appendChild(row);
-            queue.push({ file: file, row: row });
+            queue.push({ file: file, row: row, batch: batch, seq: i });
         });
         pump();
     }
@@ -86,7 +99,7 @@
 
     function run(job) {
         active++;
-        uploadOne(job.file, job.row)
+        uploadOne(job)
             .then(function () { setState(job.row, 'done', 'done'); bumpCount(); })
             .catch(function (err) {
                 setState(job.row, 'failed', 'error');
@@ -236,7 +249,9 @@
         return name.replace(/\.[^.\/]*$/, '') + '.jpg';
     }
 
-    function uploadOne(file, row) {
+    function uploadOne(job) {
+        var file = job.file;
+        var row = job.row;
         var bar = row.querySelector('.bar i');
         setState(row, maxRes ? 'resizing' : 'thumbnail');
 
@@ -248,6 +263,10 @@
             var form = new FormData();
             form.append('slug', slug);
             if (uploadKey) form.append('key', uploadKey);
+            // Kept on the job, so a manual retry lands in its original place
+            // even when the files after it are already stored.
+            form.append('batch', job.batch);
+            form.append('seq', String(job.seq));
             if (out.resized) form.append('original', out.resized, jpegName(file.name));
             else form.append('original', file, file.name);
             if (out.thumb) form.append('thumb', out.thumb, 'thumb.jpg');

+ 13 - 3
docs/ARCHITECTURE.md

@@ -102,7 +102,7 @@ admin.js                         api.php                    Hetzner S3
    │  POST multipart (original + thumb, one file) ─▶ │
    │                                                 │  PUT original ─────▶
    │                                                 │  PUT thumb ────────▶
-   │                                                 │  append to gallery JSON
+   │                                                 │  store in gallery JSON
    │ ◀──────────────────────── { ok, key, thumb, count }
 ```
 
@@ -134,18 +134,28 @@ root).
 **Why parallel.** Each request is store-and-forward: PHP buffers the whole body
 to a temp file before `s3_put_file()` starts, so during the webhost→S3 leg (and
 during every thumbnail decode) the browser's uplink sits idle. Overlapping a few
-requests keeps it saturated. Three things make that safe rather than merely
+requests keeps it saturated. Four things make that safe rather than merely
 faster:
 
 - Both endpoints call `session_write_close()` right after authenticating. PHP
   holds an exclusive lock on the session file for the whole request, so without
   it every parallel upload would queue behind the previous one and the uploader
   would be serial again regardless of how many requests it starts.
-- The gallery entry is appended via `gallery_append_image()` →`json_update()`,
+- The gallery entry is stored via `gallery_append_image()` →`json_update()`,
   which holds `flock(LOCK_EX)` on a sidecar `<file>.lock` across the whole
   read-modify-write. (The lock cannot live on the JSON file itself: `json_write()`
   replaces it by `rename()`, so the inode changes on every write.) Unlocked,
   eight simultaneous appends lose about five of them.
+- Gallery order is array order, and uploads finish in an order set by file size
+  and network luck — so the entry is *placed*, not appended. `admin.js` tags each
+  selection (one drop, or one trip through the file dialog) with a random
+  `batch` id and each file with its `seq` within it; `gallery_image_position()`
+  puts the arrival next to its siblings. A batch therefore occupies one
+  contiguous run: the first arrival appends at the end, later ones insert inside
+  that run, so a second selection dropped mid-upload stays separate and in
+  order, and a manual retry rejoins its original place. Entries stored before
+  this existed carry no `batch` and are never moved; an upload without usable
+  ordering (an older cached `admin.js`) simply appends.
 - Transient failures are retried on both sides — up to 3 attempts with backoff
   in `s3_put_file()` (re-signed and rewound per attempt) and in `admin.js` for
   network errors, 408, 429 and 5xx. 4xx is a real rejection and is never

+ 3 - 0
docs/SETUP.md

@@ -45,6 +45,9 @@ a tight per-site process limit, lower it:
 'concurrency' => 2,   // or 1 to restore strictly serial uploads
 ```
 
+Gallery order does not depend on this: files are stored in the order they were
+selected whatever value you set, and whatever order the uploads finish in.
+
 If uploads start failing with 503s under load, that limit is the first thing to
 check.
 

+ 5 - 2
upload-api.php

@@ -4,7 +4,9 @@
  * on upload.php. Same one-multipart-POST-per-image contract as admin/api.php, but
  * authenticated by the per-gallery upload key instead of an admin session.
  *
- * Fields: slug, key, original (required), thumb (optional). Access requires the
+ * Fields: slug, key, original (required), thumb, batch, seq (optional; batch and
+ * seq carry the file's place in the visitor's selection, so parallel uploads are
+ * stored in the order they were picked). Access requires the
  * gallery to have guest uploads enabled, the key to match, the gallery to be
  * unexpired, and — if the gallery has a password — the visitor to have unlocked
  * it in this session (via upload.php). Any failure returns a uniform 403.
@@ -55,6 +57,7 @@ if (!$authorized) {
     $gallery,
     $_FILES['original'] ?? null,
     $_FILES['thumb'] ?? null,
-    true
+    true,
+    $_POST
 );
 json_response($payload, $status);