Skip to content

updateFilesPath() blind UPDATE breaks on the uk_ecm_files unique key #21

Description

@Pichinov-Jose

Repository: SplashSync/Dolibarr
Module version: 2.23.3
Dolibarr: 23.0.0
Related: see the companion issue on product reference sanitisation — both come from using the raw, unsanitised reference.

Summary

When a product reference changes, ImagesTrait::updateFilesPath() moves the ECM rows to the new
folder with a single unconditional UPDATE. If the destination folder already holds a row with the
same filename, the write violates uk_ecm_files (filepath, filename, entity) and the whole product
write fails.

Error

Splash\Local\Objects\Product::updateFilesPath() => Error :
  Duplicate entry 'produit/SAM-BIK2-3-COL-1200-25-N-Photo-2171-e1709925472146.jpg-1'
  for key 'uk_ecm_files'

Root cause

src/Core/ImagesTrait.php:308

$sql = $sql = 'UPDATE '.MAIN_DB_PREFIX.$ecmImage->table_element.' SET';
$sql .= ' filepath = "'.$element.'/'.$newRef.'" ';
$sql .= ' WHERE filepath="'.$element.'/'.$oldRef.'"';

Nothing checks whether <element>/<newRef> already contains a row with the same filename.
This is common in practice: shops routinely reuse a single illustration across many products.
On our install one image was attached to 10+ course products, so any rename into an already
populated folder failed.

(Minor: $sql = $sql = ... on that line is a duplicated assignment.)

Steps to reproduce

  1. Two products share an image with the same filename.
  2. Rename product A so its folder becomes product B's existing folder path — or, more simply, rename
    a product twice so an ECM row already exists at the destination.
  3. Sync: the update aborts, the product write fails.

Suggested fix

Move row by row and drop the source row when the destination already holds that filename:

$oldPath = $element.'/'.$oldRef;
$newPath = $element.'/'.$newRef;

$sql = 'SELECT rowid, filename, entity FROM '.MAIN_DB_PREFIX.$ecmImage->table_element
     ." WHERE filepath = '".$db->escape($oldPath)."'";
$resql = $db->query($sql);
while ($resql && ($row = $db->fetch_object($resql))) {
    $exists = $db->query(
        'SELECT rowid FROM '.MAIN_DB_PREFIX.$ecmImage->table_element
        ." WHERE filepath = '".$db->escape($newPath)."'"
        ." AND filename = '".$db->escape($row->filename)."'"
        ." AND entity = ".((int) $row->entity)
    );
    if ($exists && $db->num_rows($exists) > 0) {
        // destination already documented: drop the superseded source row
        $db->query('DELETE FROM '.MAIN_DB_PREFIX.$ecmImage->table_element
            .' WHERE rowid = '.((int) $row->rowid));
        continue;
    }
    $db->query('UPDATE '.MAIN_DB_PREFIX.$ecmImage->table_element
        ." SET filepath = '".$db->escape($newPath)."' WHERE rowid = ".((int) $row->rowid));
}

A quicker one-liner would be UPDATE IGNORE, but it leaves the source rows behind as orphans —
which is the second half of the problem below.

Related: orphan rows accumulate

updateFilesPath() only runs when Splash itself sees the reference change. Every reference change
from any other source (manual edit, import, another module) leaves the old ECM rows in place. On our
install this had accumulated to 112 orphan folders / 735 rows, of which 96 folders no longer
existed on disk at all
. Each one is a latent collision for a future rename.

Worth considering, in decreasing order of scope:

  • a read-only check in the existing self-tests (admin/ServerTests.php) reporting the orphan count;
  • a generic orphan-ECM cleaner — arguably this belongs in Dolibarr core (htdocs/admin/tools/)
    rather than in a connector, since the problem is not specific to Splash.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions