recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

PhabricatorPeopleProfilePictureController: fix "Duplicate entry" error

Summary:
"Thanks" to a recent change designed to keep track of transformed images
(ee4245098b9f4cb996501631d94de07840db2620) it's more evident that
Phorge was creating duplicate PhabricatorFile(s).

These duplicate files - now tracked - were causing this regression:

#1062: Duplicate entry 'PHID-FILE-ffps4pzs36kkikrpvgdh-profile' for key 'originalPHID'

This crash was reproduced in the "Edit Profile Picture" menu, when selecting
your GitHub profile picture.

After this patch, the original image has the "profile transform" generated only once
in your database. So, consequent "Edit Profile Picture" on the same GitHub
image does not pollute your database table 'phabricator_file.file' with duplicates,
and this error is fixed.

See also D26803 with a similar bug by @aklapper, during the early
phase of the GitHub image import.

Closes T16527
Ref T16528

Test Plan:
Prepare GitHub OAuth (curtesy of aklapper):

* Log into github.com create a new OAuth App via https://github.com/settings/applications/new
* As an admin, set up GitHub Auth provider via http://phorge.localhost/auth/
* Copy Client ID into "OAuth App ID" and Client Secret into "OAuth App Secret"

Then:

* visit profile Settings > External Accounts, and connect GitHub
* visit profile Manage > Edit Profile Picture, and select the GitHub image

The error `#1062: Duplicate entry 'PHID-FILE-foobar' for key 'originalPHID'` does not appear.

Check for regressions:

Upload a new image as profile picture: it still works.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, tobiaswiese, Matthew, aklapper, Cigaryno

Maniphest Tasks: T16528, T16527

Differential Revision: https://we.phorge.it/D26804

Valerio Bozzolan 088d1f61 ec7a82e5

+45 -2
+43
src/applications/files/transform/PhabricatorFileTransform.php
··· 15 15 return array($this); 16 16 } 17 17 18 + /** 19 + * Get an existing transformed file, or create a new transformed file if no 20 + * transformed file already exists. 21 + * If a new file is produced, it is connected to the original file 22 + * in an explicit way, so, persisting a new 'PhabricatorTransformedFile' row. 23 + * 24 + * @param PhabricatorFile $file Original file. 25 + * You must check yourself if the viewer has 26 + * sufficient permissions to see this file. 27 + * @return PhabricatorFile Transformed file 28 + */ 29 + public function getOrExecuteTransformExplicit(PhabricatorFile $file) { 30 + // Use of omnipotent user is okay here because the assume 31 + // the user can see the input $file, and so, its transforms. 32 + // See PhabricatorFile::hasAutomaticCapability(). 33 + $xformed_file = id(new PhabricatorFileQuery()) 34 + ->setViewer(PhabricatorUser::getOmnipotentUser()) 35 + ->withTransforms( 36 + array( 37 + array( 38 + 'originalPHID' => $file->getPHID(), 39 + 'transform' => $this->getTransformKey(), 40 + ), 41 + )) 42 + ->executeOne(); 43 + 44 + if ($xformed_file) { 45 + return $xformed_file; 46 + } 47 + 48 + return $this->executeTransformExplicit($file); 49 + } 50 + 51 + /** 52 + * Create a new transformed file. 53 + * This usually causes the creation of a new 'PhabricatorFile'. 54 + * 55 + * @param PhabricatorFile $file Original file 56 + * @return PhabricatorFile Transformed file 57 + */ 18 58 public function executeTransform(PhabricatorFile $file) { 19 59 if ($this->canApplyTransform($file)) { 20 60 try { ··· 30 70 /** 31 71 * Wrapper of executeTransform() that also persists the relationship 32 72 * between the original file and the transform, if it makes sense to do so. 73 + * 74 + * @param PhabricatorFile $file Original file 75 + * @return PhabricatorFile Transformed file 33 76 */ 34 77 public function executeTransformExplicit(PhabricatorFile $file) { 35 78 // This can be NULL.
+1 -1
src/applications/people/controller/PhabricatorPeopleProfilePictureController.php
··· 76 76 $e_file = pht('Not Supported'); 77 77 $errors[] = $supported_formats_message; 78 78 } else { 79 - $xformed = $xform->executeTransformExplicit($file); 79 + $xformed = $xform->getOrExecuteTransformExplicit($file); 80 80 } 81 81 } 82 82
+1 -1
src/infrastructure/lint/linter/__tests__/map.php
··· 15 15 'unnecessary-dependency.lint-test' => '1608e0c1', 16 16 ), 17 17 'symbols' => array( 18 - 'javelin-test' => 'e575f333', 18 + 'javelin-test' => '3f966f42', 19 19 ), 20 20 'requires' => array( 21 21 '1608e0c1' => array(