Skip to content

Commit 3ca7c38

Browse files
authored
Merge pull request #4544 from nextcloud/fix/catch-failing-getUser
Shares fixes, refactoring and optimizations
2 parents e619509 + 5f84c00 commit 3ca7c38

39 files changed

Lines changed: 694 additions & 422 deletions

lib/Command/Share/Add.php

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@
1212
use OCA\Polls\AppInfo\Application;
1313
use OCA\Polls\Db\Poll;
1414
use OCA\Polls\Exceptions\ShareAlreadyExistsException;
15-
use OCA\Polls\Model\Group\Group;
16-
use OCA\Polls\Model\User\Email;
17-
use OCA\Polls\Model\User\User;
15+
use OCA\Polls\Model\UserBase;
1816
use OCP\AppFramework\Db\DoesNotExistException;
1917
use Stecman\Component\Symfony\Console\BashCompletion\CompletionContext;
2018
use Symfony\Component\Console\Input\InputArgument;
@@ -86,7 +84,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
8684
private function inviteUsers(Poll $poll, array $userIds): void {
8785
foreach ($userIds as $userId) {
8886
try {
89-
$share = $this->shareService->add($poll->getId(), User::TYPE, $userId);
87+
$share = $this->shareService->add($poll->getId(), UserBase::TYPE_USER, $userId);
9088
$this->shareService->sendInvitation($share);
9189
} catch (ShareAlreadyExistsException $e) {
9290
// silently ignore already existing shares
@@ -102,7 +100,7 @@ private function inviteUsers(Poll $poll, array $userIds): void {
102100
private function inviteGroups(Poll $poll, array $groupIds): void {
103101
foreach ($groupIds as $groupId) {
104102
try {
105-
$share = $this->shareService->add($poll->getId(), Group::TYPE, $groupId);
103+
$share = $this->shareService->add($poll->getId(), UserBase::TYPE_GROUP, $groupId);
106104
$this->shareService->sendInvitation($share);
107105
} catch (ShareAlreadyExistsException $e) {
108106
// silently ignore already existing shares
@@ -118,7 +116,7 @@ private function inviteGroups(Poll $poll, array $groupIds): void {
118116
private function inviteEmails(Poll $poll, array $emails): void {
119117
foreach ($emails as $email) {
120118
try {
121-
$share = $this->shareService->add($poll->getId(), Email::TYPE, $email);
119+
$share = $this->shareService->add($poll->getId(), UserBase::TYPE_EMAIL, $email);
122120
$this->shareService->sendInvitation($share);
123121
} catch (ShareAlreadyExistsException $e) {
124122
// silently ignore already existing shares

lib/Command/Share/Remove.php

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,7 @@
1212
use OCA\Polls\AppInfo\Application;
1313
use OCA\Polls\Db\Poll;
1414
use OCA\Polls\Db\Share;
15-
use OCA\Polls\Model\Group\Group;
16-
use OCA\Polls\Model\User\Contact;
17-
use OCA\Polls\Model\User\Email;
18-
use OCA\Polls\Model\User\GenericUser;
19-
use OCA\Polls\Model\User\User;
15+
use OCA\Polls\Model\UserBase;
2016
use OCP\AppFramework\Db\DoesNotExistException;
2117
use Symfony\Component\Console\Input\InputArgument;
2218
use Symfony\Component\Console\Input\InputInterface;
@@ -126,7 +122,7 @@ private function removeEmails(Poll $poll, array $emails): void {
126122
private function getUserShares(Poll $poll): array {
127123
$shares = $this->shareMapper->findByPoll($poll->getId());
128124
return array_values(array_filter($shares, static function (Share $share): bool {
129-
return ($share->getType() === User::TYPE);
125+
return ($share->getType() === UserBase::TYPE_USER);
130126
}));
131127
}
132128

@@ -138,7 +134,7 @@ private function getUserShares(Poll $poll): array {
138134
private function getGroupShares(Poll $poll): array {
139135
$shares = $this->shareMapper->findByPoll($poll->getId());
140136
return array_values(array_filter($shares, static function (Share $share): bool {
141-
return ($share->getType() === Group::TYPE);
137+
return ($share->getType() === UserBase::TYPE_GROUP);
142138
}));
143139
}
144140

@@ -150,11 +146,11 @@ private function getGroupShares(Poll $poll): array {
150146
private function getEmailShares(Poll $poll): array {
151147
$shares = $this->shareMapper->findByPoll($poll->getId());
152148
return array_values(array_filter($shares, static function (Share $share): bool {
153-
if (($share->getType() === GenericUser::TYPE) && $share->getEmailAddress()) {
149+
if (($share->getType() === UserBase::TYPE_GENERIC) && $share->getEmailAddress()) {
154150
return true;
155151
}
156152

157-
return (($share->getType() === Email::TYPE) || ($share->getType() === Contact::TYPE));
153+
return (($share->getType() === UserBase::TYPE_EMAIL) || ($share->getType() === UserBase::TYPE_CONTACT));
158154
}));
159155
}
160156
}

lib/Controller/PublicController.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,21 @@ public function validatePublicDisplayName(string $displayName, string $token): J
403403
]);
404404
}
405405

406+
/**
407+
* Validate it the user name is reserved
408+
* return false, if this username already exists as a user or as a participant of the poll
409+
* @param string $token Share token
410+
*/
411+
#[PublicPage]
412+
#[ShareTokenRequired]
413+
#[OpenAPI(OpenAPI::SCOPE_IGNORE)]
414+
#[FrontpageRoute(verb: 'POST', url: '/randomname')]
415+
public function getRandomUserName(string $token): JSONResponse {
416+
return $this->response(fn () => [
417+
'name' => $this->systemService->getRandomDisplayName($token)
418+
]);
419+
}
420+
406421
/**
407422
* Validate email address (simple validation)
408423
* @param string $emailAddress Email address string to check for validation

lib/Db/Comment.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public function getRecipientUser() : ?UserBase {
6767
try {
6868
/* @var UserMapper $userMapper */
6969
$userMapper = (Container::queryClass(UserMapper::class));
70-
$user = $userMapper->getParticipant($this->getRecipient(), $this->getPollId());
70+
$user = $userMapper->getUser($this->getRecipient(), $this->getPollId());
7171
} catch (Exception $e) {
7272
return null;
7373
}

lib/Db/EntityWithUser.php

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -91,30 +91,29 @@ protected function getEntityAnonymization(): bool {
9191
return false;
9292
}
9393

94+
/**
95+
* get the pollId, if relevant for user identification, otherwise return null
96+
* Overwrite if user identificaton needs another pollId than the one of the entity
97+
*/
98+
public function getUserRelevantPollId(): ?int {
99+
return $this->getPollId();
100+
}
94101

95102
/**
96-
* @return UserBase Gets owner of the entity
103+
* @return UserBase Gets user of the entity
97104
*/
98105
public function getUser(): UserBase {
99106
if ($this->getEntityAnonymization()) {
100107
$user = new Anon($this->getUserId());
101108
return $user;
102109
}
103-
104110
/** @var UserMapper */
105-
$userMapper = (Container::queryClass(UserMapper::class));
111+
$userMapper = Container::queryClass(UserMapper::class);
106112

107113
try {
108-
$pollId = $this->getPollId();
109-
if ($pollId === null) {
110-
// If pollId is not set, we assume that the user is not a participant of a poll
111-
return $userMapper->getUserFromUserBase($this->getUserId());
112-
}
113-
$user = $userMapper->getParticipant($this->getUserId(), $pollId);
114-
// Get user from userbase
114+
return $userMapper->getUser($this->getUserId(), $this->getUserRelevantPollId());
115115
} catch (Exception) {
116-
$user = new Ghost($this->getUserId());
116+
return new Ghost($this->getUserId());
117117
}
118-
return $user;
119118
}
120119
}

lib/Db/Poll.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,13 @@ public function getUserId(): string {
516516
return $this->getOwner();
517517
}
518518

519+
/**
520+
* Return null for user identification for the poll user to support caching
521+
*/
522+
public function getUserRelevantPollId(): ?int {
523+
return null;
524+
}
525+
519526
// alias of setOwner($value)
520527
public function setUserId(string $userId): void {
521528
$this->setOwner($userId);

lib/Db/Share.php

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@
4848
* @method void setLocked(int $value)
4949
* @method string getDisplayName()
5050
* @method void setDisplayName(string $value)
51+
* @method string getLabel()
52+
* @method void setLabel(string $value)
5153
* @method string getMiscSettings()
5254
* @method void setMiscSettings(string $value)
5355
* @method ?int getAnonymizedVotes()
@@ -63,17 +65,17 @@ class Share extends EntityWithUser implements JsonSerializable {
6365
public const EMAIL_DISABLED = 'disabled';
6466

6567
// Only authenticated access
66-
public const TYPE_USER = 'user';
67-
public const TYPE_ADMIN = 'admin';
68-
public const TYPE_GROUP = 'group';
68+
public const TYPE_USER = UserBase::TYPE_USER;
69+
public const TYPE_ADMIN = UserBase::TYPE_ADMIN;
70+
public const TYPE_GROUP = UserBase::TYPE_GROUP;
6971

7072
// Public and authenticated Access
71-
public const TYPE_PUBLIC = 'public';
73+
public const TYPE_PUBLIC = UserBase::TYPE_PUBLIC;
7274

7375
// Only public access
74-
public const TYPE_EMAIL = 'email';
75-
public const TYPE_CONTACT = 'contact';
76-
public const TYPE_EXTERNAL = 'external';
76+
public const TYPE_EMAIL = UserBase::TYPE_EMAIL;
77+
public const TYPE_CONTACT = UserBase::TYPE_CONTACT;
78+
public const TYPE_EXTERNAL = UserBase::TYPE_EXTERNAL;
7779

7880
// no direct Access
7981
public const TYPE_TEAM = 'circle';
@@ -178,7 +180,7 @@ public function jsonSerialize(): array {
178180
}
179181

180182
public function resolveUser(): Ghost|Group|Team|Contact|ContactGroup|User|Email|GenericUser {
181-
return UserMapper::getUserObject(
183+
return UserMapper::createUserObject(
182184
$this->getType(),
183185
$this->getUserId(),
184186
$this->getDisplayName(),
@@ -242,35 +244,50 @@ public function setPublicPollEmail(string $value): void {
242244
$this->setMiscSettingsByKey('publicPollEmail', $value);
243245
}
244246

245-
/**
246-
* Share label for public shares, now stored as displayName
247-
* TODO: remove after migration was introduced
248-
*/
249-
public function setLabel(string $label): void {
250-
$this->setDisplayName($label);
251-
$this->label = $label;
252-
}
253-
254247
/**
255248
* Share label for public shares
256249
* TODO: remove after migrating labels to displayName
257250
*/
258251
public function getLabel(): string {
259252
// In case of public poll use label as fallback for displayName
260-
return $this->displayName ?? $this->label ?? '';
253+
return $this->label ?? $this->displayName ?? '';
261254
}
262255

263256
/**
264257
* Sharee's displayName. In case of public poll label is used instead
265258
* TODO: remove public poll check after migration labels to displayName
266259
*/
267260
public function getDisplayName(): string {
268-
if ($this->getType() === self::TYPE_PUBLIC) {
269-
return $this->getLabel();
270-
}
271261
return $this->displayName ?? '';
272262
}
273263

264+
public function setFromUserObject(UserBase $userGroup, string $token, string $purpose = 'poll'): void {
265+
// Contacts are converted: with email → email share, without email → external share
266+
$type = $userGroup->getType();
267+
if ($type === self::TYPE_CONTACT || $type === self::TYPE_EMAIL) {
268+
$type = self::TYPE_EXTERNAL;
269+
}
270+
271+
$this->setType($type);
272+
$this->setDisplayName($userGroup->getDisplayName());
273+
$this->setEmailAddress($userGroup->getEmailAddress());
274+
275+
// ignore if share is for poll Groups
276+
if ($purpose === 'poll') {
277+
if ($type === self::TYPE_PUBLIC) {
278+
// public share to create, set token as userId
279+
$this->setUserId($token);
280+
} else {
281+
$this->setUserId($userGroup->getShareUserId());
282+
}
283+
if ($type === self::TYPE_EXTERNAL) {
284+
$this->setLanguage($userGroup->getLanguageCode());
285+
$this->setTimeZoneName($userGroup->getTimeZoneName());
286+
}
287+
}
288+
}
289+
290+
274291
public function getTimeZoneName(): string {
275292
return $this->getMiscSettingsArray()['timeZone'] ?? '';
276293
}

0 commit comments

Comments
 (0)