[full-ci] Accept OCM to groups

[michielbdejong]

Description

These are the hooks we need in the core repo to allow the FederatedGroups app to deal with outgoing and incoming OCM shares to group.

Related Issue

• Fixes OCM sharing to groups not supported? #40530

Motivation and Context

How Has This Been Tested?

• test environment:
• test case 1:
• test case 2:
• ...

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:
• Changelog item, see TEMPLATE

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
4 out of 6 committers have signed the CLA.

✅ phil-davis
✅ jnweiger
✅ DeepDiver1975
✅ shokri-navid
❌ ownclouders
❌ Pasquale Tripodi

---

Pasquale Tripodi seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[jnweiger]

@michielbdejong CI is picky about coding standards:

--- /drone/src/apps/files_sharing/lib/Controllers/ExternalSharesController.php
+++ /drone/src/apps/files_sharing/lib/Controllers/ExternalSharesController.php
@@ -39,7 +39,6 @@
  * @package OCA\Files_Sharing\Controllers
  */
 class ExternalSharesController extends Controller {
-
 	/** @var \OCA\Files_Sharing\External\Manager */
 	private $externalManager;
 	/** @var \OCA\Files_Sharing\External\Manager */
@@ -87,7 +86,7 @@
 		if ($this->groupExternalManager !== null) {
 			$federatedGroupResult = $this->groupExternalManager->getOpenShares();
 		}
-		$result = array_merge($federatedGroupResult,  $this->externalManager->getOpenShares());
+		$result = array_merge($federatedGroupResult, $this->externalManager->getOpenShares());
 		return new JSONResponse($result);
 	}

[phil-davis]

@michielbdejong https://drone.owncloud.com/owncloud/core/38230/3/7
It still fails code-style checks. If you fix those very minor things then we will get to see the results of unit tests and acceptance tests.

https://drone.owncloud.com/owncloud/core/38230/6/8
You might also need to add some "ignores" to phpstan.neon for the Access to an undefined property messages.

[ownclouders]

💥 Acceptance tests pipeline webUIFileActionsMenu-chrome-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/38313/151

[jvillafanez]

Just saying that the commits should be squashed when the PR is ready to merge.

[michielbdejong]

Hm, build was killed, I'm not sure why.

[jvillafanez]

There are some tests failing in https://drone.owncloud.com/owncloud/core/38306/148/18

{"reqId":"hKNpkBk8bIqVRcbEE0R2","level":4,"time":"2023-04-19T09:20:55+00:00","remoteAddr":"192.168.28.5","user":"Alice","app":"webdav","method":"PROPFIND","url":"\/remote.php\/dav\/files\/Alice\/","message":"Exception: No share provider for share type 7: {\"Exception\":\"OC\\\\Share20\\\\Exception\\\\ProviderException\",\"Message\":\"No share provider for share type 7\",\"Code\":0,\"Trace\":\"#0 \\\/drone\\\/src\\\/lib\\\/private\\\/Share20\\\/Manager.php(160): OC\\\\Share20\\\\ProviderFactory->getProviderForType()\\n#1 \\\/drone\\\/src\\\/lib\\\/private\\\/Share20\\\/Manager.php(1272): OC\\\\Share20\\\\Manager->shareTypeToProviderMap()\\n#2 \\\/drone\\\/src\\\/apps\\\/dav\\\/lib\\\/Connector\\\/Sabre\\\/SharesPlugin.php(146): OC\\\\Share20\\\\Manager->getAllSharesBy()\\n#3 \\\/drone\\\/src\\\/apps\\\/dav\\\/lib\\\/Connector\\\/Sabre\\\/SharesPlugin.php(197): OCA\\\\DAV\\\\Connector\\\\Sabre\\\\SharesPlugin->getSharesForNodeIds()\\n#4 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/event\\\/lib\\\/WildcardEmitterTrait.php(89): OCA\\\\DAV\\\\Connector\\\\Sabre\\\\SharesPlugin->handleGetProperties()\\n#5 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(1052): Sabre\\\\DAV\\\\Server->emit()\\n#6 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(984): Sabre\\\\DAV\\\\Server->getPropertiesByNode()\\n#7 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(1662): Sabre\\\\DAV\\\\Server->getPropertiesIteratorForPath()\\n#8 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(1642): Sabre\\\\DAV\\\\Server->writeMultiStatus()\\n#9 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/http\\\/lib\\\/Sapi.php(83): Sabre\\\\DAV\\\\Server->Sabre\\\\DAV\\\\{closure}(*** sensitive parameters replaced ***)\\n#10 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(490): Sabre\\\\HTTP\\\\Sapi::sendResponse()\\n#11 \\\/drone\\\/src\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(253): Sabre\\\\DAV\\\\Server->invokeMethod()\\n#12 \\\/drone\\\/src\\\/apps\\\/dav\\\/lib\\\/Server.php(348): Sabre\\\\DAV\\\\Server->start()\\n#13 \\\/drone\\\/src\\\/apps\\\/dav\\\/appinfo\\\/v2\\\/remote.php(31): OCA\\\\DAV\\\\Server->exec()\\n#14 \\\/drone\\\/src\\\/remote.php(165): require_once('\\\/drone\\\/src\\\/apps...')\\n#15 {main}\",\"File\":\"\\\/drone\\\/src\\\/lib\\\/private\\\/Share20\\\/ProviderFactory.php\",\"Line\":138}"}
{"reqId":"hKNpkBk8bIqVRcbEE0R2","level":3,"time":"2023-04-19T09:20:55+00:00","remoteAddr":"192.168.28.5","user":"Alice","app":"PHP","method":"PROPFIND","url":"\/remote.php\/dav\/files\/Alice\/","message":"Cannot modify header information - headers already sent by (output started at \/drone\/src\/lib\/composer\/sabre\/http\/lib\/Message.php:117) at \/drone\/src\/lib\/composer\/sabre\/http\/lib\/Sapi.php#66"}
{"reqId":"hKNpkBk8bIqVRcbEE0R2","level":3,"time":"2023-04-19T09:20:55+00:00","remoteAddr":"192.168.28.5","user":"Alice","app":"PHP","method":"PROPFIND","url":"\/remote.php\/dav\/files\/Alice\/","message":"Cannot modify header information - headers already sent by (output started at \/drone\/src\/lib\/composer\/sabre\/http\/lib\/Message.php:117) at \/drone\/src\/lib\/composer\/sabre\/http\/lib\/Sapi.php#70"}
{"reqId":"hKNpkBk8bIqVRcbEE0R2","level":3,"time":"2023-04-19T09:20:55+00:00","remoteAddr":"192.168.28.5","user":"Alice","app":"PHP","method":"PROPFIND","url":"\/remote.php\/dav\/files\/Alice\/","message":"Cannot modify header information - headers already sent by (output started at \/drone\/src\/lib\/composer\/sabre\/http\/lib\/Message.php:117) at \/drone\/src\/lib\/composer\/sabre\/http\/lib\/Sapi.php#70"}
{"reqId":"hKNpkBk8bIqVRcbEE0R2","level":3,"time":"2023-04-19T09:20:55+00:00","remoteAddr":"192.168.28.5","user":"Alice","app":"PHP","method":"PROPFIND","url":"\/remote.php\/dav\/files\/Alice\/","message":"Cannot modify header information - headers already sent by (output started at \/drone\/src\/lib\/composer\/sabre\/http\/lib\/Message.php:117) at \/drone\/src\/lib\/composer\/sabre\/http\/lib\/Sapi.php#70"}

[phil-davis]

The UI acceptance test failures were in just one pipeline, and somehow user Alice could not log in. That looks not directly related to this PR. So I have restarted CI to see if it will pass.

Note that for ordinary code PRs like this, the whole drone build is cancelled as soon as one acceptance test pipeline fails. That is done to save wasted resources when something fails anyway.

If a PR has a few different problems that cause tests to fail in multiple pipelines, then by default you only get to see the first pipeline fail. If you want to be able to see all the failures then put full-ci in the PR title. The drone CI code looks for that, and in that case it lets all pipelines run to completion before the build fails. That can be useful to be able to see all the problems in a single CI run.

[navid-shokri]

@phil-davis
I fixed all the tests locally but Pipelines are failing. we cannot find the root cause of this. is it an infrastructure failure or are there some errors on our side? would you be able to review the process and let us know if there are any points to regard?

[phil-davis]

Now there are 220 commits in this PR - what happened???

[T0mWz]

When appling this patch, I got;

Exception: No share provider for share type 7

Maybe good to verify behavior without Sciencemesh app, or it should be default shipped.

[phil-davis]

Now there are 220 commits in this PR - what happened???

I cherry-picked what I think are the real commits for this PR and created PR #40877 - that should be easier to review, rather than whatever happened here to have 221 commits listed.

[jvillafanez]

Left a comment in https://github.com/owncloud/core/pull/40877/files#r1265137966 , but for the rest the other PR looks good.

When appling this patch, I got;
Exception: No share provider for share type 7
Maybe good to verify behavior without Sciencemesh app, or it should be default shipped.

Maybe a problem in https://github.com/owncloud/core/pull/40877/files#diff-6c825986578290fe355bffe05c5232629a36e554d48f5d889bb2c1ec73645265R1542
I think we'll have to check what providers are available before trying to get an optional one.

Now there are 220 commits in this PR - what happened???

We need to decide how to move forward. Options are:

• They fix the branch on their side
• They open a new PR with the relevant changes. Basically Ocm to groups 20230717 #40877 but on their side so they can keep applying the changes needed.
• We take over the PR from Ocm to groups 20230717 #40877 . This doesn't seem a good solution because I don't think we can test the whole solution, just that it doesn't break if the app isn't installed.

[phil-davis]

@michielbdejong @shokri-navid
was this done in #40886 ?

Can we close this PR?

[jvillafanez]

I think this is the old PR that got broken and was superseded by the one that was already merged, but we can wait for confirmation.

[michielbdejong]

Superseded by #40886