Add share type to the verifyExpirationDate hook

[VicDeo]

Description

add type of the share to the verifyExpirationDate hook

Related Issue

• Part of the fix for Cannot create ordinary share with users when public link expiry is set password_policy#287

Motivation and Context

it is not possible to distinguish public links from other share types

How Has This Been Tested?

1. Set public link expiration policies in the password policy app according to Cannot create ordinary share with users when public link expiry is set password_policy#287

2. Create a user or group share

expected

share is created

actual

share creation is blocked due to public link policies

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

[phil-davis]

This looks like "a good thing" (tm). Can a changelog be added? - done.

In the morning I can make some acceptance tests in password_policy that demonstrate the bug and then that the fix works.

[codecov]

Codecov Report

Merging #37135 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master   #37135      +/-   ##
============================================
+ Coverage     64.78%   64.78%   +<.01%     
  Complexity    19130    19130              
============================================
  Files          1267     1267              
  Lines         74912    74913       +1     
  Branches       1328     1328              
============================================
+ Hits          48533    48534       +1     
  Misses        25989    25989              
  Partials        390      390

Flag	Coverage Δ	Complexity Δ
#javascript	54.18% <ø> (ø)	0 <ø> (ø)	⬇️
#phpunit	65.96% <100%> (ø)	19130 <0> (ø)	⬇️

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Share20/Manager.php	96.31% <100%> (ø)	273 <0> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ccfb9e1...bc7aad7. Read the comment docs.

[codecov]

Codecov Report

Merging #37135 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master   #37135   +/-   ##
=========================================
  Coverage     64.78%   64.78%           
  Complexity    19130    19130           
=========================================
  Files          1267     1267           
  Lines         74912    74913    +1     
  Branches       1328     1328           
=========================================
+ Hits          48533    48534    +1     
  Misses        25989    25989           
  Partials        390      390           

Flag	Coverage Δ	Complexity Δ
#javascript	54.18% <ø> (ø)	0.00 <ø> (ø)
#phpunit	65.96% <100.00%> (+<0.01%)	19130.00 <0.00> (ø)

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Share20/Manager.php	96.31% <100.00%> (+<0.01%)	273.00 <0.00> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ccfb9e1...e4d16f0. Read the comment docs.

[phil-davis]

@VicDeo I added a changelog.

What about unit test changes/additions?

For example, https://github.com/owncloud/core/blob/master/tests/lib/Share20/ManagerTest.php#L1130 could have:

$share->setShareType(\OCP\Share::SHARE_TYPE_LINK);

and the expects for listener can be:

		$hookListner->expects($this->once())->method('listener')->with($this->callback(function ($data) use ($expected) {
			return $data['expirationDate'] == $expected && $data['passwordSet'] === false && $data['shareType'] == \OCP\Share::SHARE_TYPE_LINK;
		}));

The existing tests (which are really for public links) could be adjusted so they explicitly setShareType.

And add some tests to check what happens to the hook when share type is SHARE_TYPE_USER and SHARE_TYPE_GROUP ?

Whatever you think.

For me it would be nice to get this PR merged ASAP. It makes drone CI testing from password_policy app much easier.

[phil-davis]

Tested locally/manually along with owncloud/password_policy#293 and works.

[VicDeo]

@phil-davis this oneliner is not worthy an unit test for me.
The coverage is already at 100% and removing this line will break acceptance tests anyway.

[phil-davis]

This has been discussed and tested - approving.