Skip to content

Commit 38f987e

Browse files
committed
fix: don't rely on constraint for filecache_extended "upsert" when in transaction
Signed-off-by: Robin Appelman <robin@icewind.nl>
1 parent 06a8aa9 commit 38f987e

1 file changed

Lines changed: 66 additions & 42 deletions

File tree

lib/private/Files/Cache/Cache.php

Lines changed: 66 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -322,17 +322,7 @@ public function insert($file, array $data) {
322322
if ($builder->executeStatement()) {
323323
$fileId = $builder->getLastInsertId();
324324

325-
if (count($extensionValues)) {
326-
$query = $this->getQueryBuilder();
327-
$query->insert('filecache_extended');
328-
$query->hintShardKey('storage', $storageId);
329-
330-
$query->setValue('fileid', $query->createNamedParameter($fileId, IQueryBuilder::PARAM_INT));
331-
foreach ($extensionValues as $column => $value) {
332-
$query->setValue($column, $query->createNamedParameter($value));
333-
}
334-
$query->executeStatement();
335-
}
325+
$this->setExtensionValues($fileId, $extensionValues, true);
336326

337327
$event = new CacheEntryInsertedEvent($this->storage, $file, $fileId, $storageId);
338328
$this->eventDispatcher->dispatch(CacheEntryInsertedEvent::class, $event);
@@ -400,47 +390,81 @@ public function update($id, array $data) {
400390
$query->executeStatement();
401391
}
402392

403-
if (count($extensionValues)) {
404-
try {
405-
$query = $this->getQueryBuilder();
406-
$query->insert('filecache_extended');
407-
$query->hintShardKey('storage', $this->getNumericStorageId());
393+
$this->setExtensionValues($id, $extensionValues, false);
408394

409-
$query->setValue('fileid', $query->createNamedParameter($id, IQueryBuilder::PARAM_INT));
410-
foreach ($extensionValues as $column => $value) {
411-
$query->setValue($column, $query->createNamedParameter($value));
412-
}
395+
$path = $this->getPathById($id);
396+
// path can still be null if the file doesn't exist
397+
if ($path !== null) {
398+
$event = new CacheEntryUpdatedEvent($this->storage, $path, $id, $this->getNumericStorageId());
399+
$this->eventDispatcher->dispatchTyped($event);
400+
}
401+
}
413402

414-
$query->executeStatement();
403+
private function hasExtensionValues(int $id) {
404+
$query = $this->getQueryBuilder();
405+
$query->select('fileid')
406+
->from('filecache_extended')
407+
->whereFileId($id);
408+
409+
return $query->executeQuery()->fetchOne() !== false;
410+
}
411+
412+
private function setExtensionValues(int $id, array $extensionValues, bool $newFile) {
413+
if (!$extensionValues) {
414+
return;
415+
}
416+
417+
// a failed insert in a transaction aborts the transactions on some platforms, so we can't rely on
418+
// that behavior to to an "upsert"
419+
// for new files, we can safely assume that there won't be a conflict, since the fileid is new
420+
if (!$newFile && $this->connection->inTransaction()) {
421+
if ($this->hasExtensionValues($id)) {
422+
$this->updateExtensionValues($id, $extensionValues);
423+
} else {
424+
$this->insertExtensionValues($id, $extensionValues);
425+
}
426+
} else {
427+
try {
428+
$this->insertExtensionValues($id, $extensionValues);
415429
} catch (Exception $e) {
416430
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
417431
throw $e;
418432
}
419-
$query = $this->getQueryBuilder();
420-
$query->update('filecache_extended')
421-
->whereFileId($id)
422-
->hintShardKey('storage', $this->getNumericStorageId())
423-
->andWhere($query->expr()->orX(...array_map(function ($key, $value) use ($query) {
424-
return $query->expr()->orX(
425-
$query->expr()->neq($key, $query->createNamedParameter($value)),
426-
$query->expr()->isNull($key)
427-
);
428-
}, array_keys($extensionValues), array_values($extensionValues))));
429-
430-
foreach ($extensionValues as $key => $value) {
431-
$query->set($key, $query->createNamedParameter($value));
432-
}
433-
434-
$query->executeStatement();
433+
$this->updateExtensionValues($id, $extensionValues);
435434
}
436435
}
436+
}
437437

438-
$path = $this->getPathById($id);
439-
// path can still be null if the file doesn't exist
440-
if ($path !== null) {
441-
$event = new CacheEntryUpdatedEvent($this->storage, $path, $id, $this->getNumericStorageId());
442-
$this->eventDispatcher->dispatchTyped($event);
438+
private function insertExtensionValues(int $id, array $extensionValues) {
439+
$query = $this->getQueryBuilder();
440+
$query->insert('filecache_extended');
441+
$query->hintShardKey('storage', $this->getNumericStorageId());
442+
443+
$query->setValue('fileid', $query->createNamedParameter($id, IQueryBuilder::PARAM_INT));
444+
foreach ($extensionValues as $column => $value) {
445+
$query->setValue($column, $query->createNamedParameter($value));
443446
}
447+
448+
$query->executeStatement();
449+
}
450+
451+
private function updateExtensionValues(int $id, array $extensionValues) {
452+
$query = $this->getQueryBuilder();
453+
$query->update('filecache_extended')
454+
->whereFileId($id)
455+
->hintShardKey('storage', $this->getNumericStorageId())
456+
->andWhere($query->expr()->orX(...array_map(function ($key, $value) use ($query) {
457+
return $query->expr()->orX(
458+
$query->expr()->neq($key, $query->createNamedParameter($value)),
459+
$query->expr()->isNull($key)
460+
);
461+
}, array_keys($extensionValues), array_values($extensionValues))));
462+
463+
foreach ($extensionValues as $key => $value) {
464+
$query->set($key, $query->createNamedParameter($value));
465+
}
466+
467+
$query->executeStatement();
444468
}
445469

446470
/**

0 commit comments

Comments
 (0)