summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--lib/private/Repair/RepairUnmergedShares.php40
-rw-r--r--tests/lib/Repair/RepairUnmergedSharesTest.php66
2 files changed, 96 insertions, 10 deletions
diff --git a/lib/private/Repair/RepairUnmergedShares.php b/lib/private/Repair/RepairUnmergedShares.php
index 353877bb873..f14e98c1761 100644
--- a/lib/private/Repair/RepairUnmergedShares.php
+++ b/lib/private/Repair/RepairUnmergedShares.php
@@ -149,6 +149,44 @@ class RepairUnmergedShares implements IRepairStep {
}
/**
+ * Decide on the best target name based on all group shares and subshares,
+ * goal is to increase the likeliness that the chosen name matches what
+ * the user is expecting.
+ *
+ * For this, we discard the entries with parenthesis "(2)".
+ * In case the user also renamed the duplicates to a legitimate name, this logic
+ * will still pick the most recent one as it's the one the user is most likely to
+ * remember renaming.
+ *
+ * If no suitable subshare is found, use the least recent group share instead.
+ *
+ * @param array $groupShares group share entries
+ * @param array $subShares sub share entries, sorted by stime
+ *
+ * @return string chosen target name
+ */
+ private function findBestTargetName($groupShares, $subShares) {
+ $pickedShare = null;
+ // note subShares are sorted by stime from oldest to newest
+ foreach ($subShares as $subShare) {
+ // skip entries that have parenthesis with numbers
+ if (preg_match('/\([0-9]*\)/', $subShare['file_target']) === 1) {
+ continue;
+ }
+ // pick any share found that would match, the last being the most recent
+ $pickedShare = $subShare;
+ }
+
+ // no suitable subshare found
+ if ($pickedShare === null) {
+ // use least recent group share target instead
+ $pickedShare = $groupShares[0];
+ }
+
+ return $pickedShare['file_target'];
+ }
+
+ /**
* Fix the given received share represented by the set of group shares
* and matching sub shares
*
@@ -171,7 +209,7 @@ class RepairUnmergedShares implements IRepairStep {
return false;
}
- $targetPath = $groupShares[0]['file_target'];
+ $targetPath = $this->findBestTargetName($groupShares, $subShares);
// check whether the user opted out completely of all subshares
$optedOut = true;
diff --git a/tests/lib/Repair/RepairUnmergedSharesTest.php b/tests/lib/Repair/RepairUnmergedSharesTest.php
index fe9b3e5b96f..9fe9a073881 100644
--- a/tests/lib/Repair/RepairUnmergedSharesTest.php
+++ b/tests/lib/Repair/RepairUnmergedSharesTest.php
@@ -204,7 +204,7 @@ class RepairUnmergedSharesTest extends TestCase {
[
// #2 bogus share
// - outsider shares with group1, group2
- // - one subshare for each group share
+ // - one subshare for each group share, both with parenthesis
// - but the targets do not match when grouped
[
[Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
@@ -218,7 +218,7 @@ class RepairUnmergedSharesTest extends TestCase {
[
['/test', 31],
['/test', 31],
- // reset to original name
+ // reset to original name as the sub-names have parenthesis
['/test', 31],
['/test', 31],
// leave unrelated alone
@@ -228,6 +228,54 @@ class RepairUnmergedSharesTest extends TestCase {
[
// #3 bogus share
// - outsider shares with group1, group2
+ // - one subshare for each group share, both renamed manually
+ // - but the targets do not match when grouped
+ [
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 31],
+ // child of the previous ones
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test_renamed (1 legit paren)', 31, 0],
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test_renamed (2 legit paren)', 31, 1],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
+ ],
+ [
+ ['/test', 31],
+ ['/test', 31],
+ // reset to less recent subshare name
+ ['/test_renamed (2 legit paren)', 31],
+ ['/test_renamed (2 legit paren)', 31],
+ // leave unrelated alone
+ ['/test (4)', 31],
+ ]
+ ],
+ [
+ // #4 bogus share
+ // - outsider shares with group1, group2
+ // - one subshare for each group share, one with parenthesis
+ // - but the targets do not match when grouped
+ [
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup1', '/test', 31],
+ [Constants::SHARE_TYPE_GROUP, 123, 'recipientgroup2', '/test', 31],
+ // child of the previous ones
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test (2)', 31, 0],
+ [DefaultShareProvider::SHARE_TYPE_USERGROUP, 123, 'user2', '/test_renamed', 31, 1],
+ // different unrelated share
+ [Constants::SHARE_TYPE_GROUP, 456, 'recipientgroup1', '/test (4)', 31],
+ ],
+ [
+ ['/test', 31],
+ ['/test', 31],
+ // reset to less recent subshare name but without parenthesis
+ ['/test_renamed', 31],
+ ['/test_renamed', 31],
+ // leave unrelated alone
+ ['/test (4)', 31],
+ ]
+ ],
+ [
+ // #5 bogus share
+ // - outsider shares with group1, group2
// - one subshare for each group share
// - first subshare not renamed (as in real world scenario)
// - but the targets do not match when grouped
@@ -251,7 +299,7 @@ class RepairUnmergedSharesTest extends TestCase {
]
],
[
- // #4 bogus share:
+ // #6 bogus share:
// - outsider shares with group1, group2
// - one subshare for each group share
// - non-matching targets
@@ -276,7 +324,7 @@ class RepairUnmergedSharesTest extends TestCase {
]
],
[
- // #5 bogus share:
+ // #7 bogus share:
// - outsider shares with group1, group2
// - one subshare for each group share
// - non-matching targets
@@ -301,7 +349,7 @@ class RepairUnmergedSharesTest extends TestCase {
]
],
[
- // #6 bogus share:
+ // #8 bogus share:
// - outsider shares with group1, group2 and also user2
// - one subshare for each group share
// - one extra share entry for direct share to user2
@@ -329,7 +377,7 @@ class RepairUnmergedSharesTest extends TestCase {
]
],
[
- // #7 bogus share:
+ // #9 bogus share:
// - outsider shares with group1 and also user2
// - no subshare at all
// - one extra share entry for direct share to user2
@@ -350,7 +398,7 @@ class RepairUnmergedSharesTest extends TestCase {
]
],
[
- // #8 legitimate share with own group:
+ // #10 legitimate share with own group:
// - insider shares with both groups the user is already in
// - no subshares in this case
[
@@ -368,7 +416,7 @@ class RepairUnmergedSharesTest extends TestCase {
]
],
[
- // #9 legitimate shares:
+ // #11 legitimate shares:
// - group share with same group
// - group share with other group
// - user share where recipient renamed
@@ -392,7 +440,7 @@ class RepairUnmergedSharesTest extends TestCase {
]
],
[
- // #10 legitimate share:
+ // #12 legitimate share:
// - outsider shares with group and user directly with different permissions
// - no subshares
// - same targets
generated by cgit v1.2.3 (git 2.39.1) at 2025-01-18 05:42:22 +0000