Skip to content

Commit

Permalink
adjust internal data handling logic to fix store and load
Browse files Browse the repository at this point in the history 
- format as stored previously in oc_accounts table is kept

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
  • Loading branch information
@blizzz
blizzz committed Jun 25, 2021
1 parent f9622e8 commit 3561025
Show file tree
Hide file tree
Showing 5 changed files with 252 additions and 284 deletions.
16 changes: 10 additions & 6 deletions lib/private/Accounts/Account.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
use OCP\Accounts\IAccountPropertyCollection;
use OCP\Accounts\PropertyDoesNotExistException;
use OCP\IUser;
use RuntimeException;

class Account implements IAccount {
use TAccountsHelper;
Expand Down Expand Up @@ -116,13 +117,16 @@ public function setPropertyCollection(IAccountPropertyCollection $propertyCollec
return $this;
}

public function getPropertyCollection(string $propertyCollection): IAccountPropertyCollection {
if (!array_key_exists($propertyCollection, $this->properties)) {
throw new PropertyDoesNotExistException($propertyCollection);
public function getPropertyCollection(string $propertyCollectionName): IAccountPropertyCollection {
if (!$this->isCollection($propertyCollectionName)) {
throw new PropertyDoesNotExistException($propertyCollectionName);
}
if (!$this->properties[$propertyCollection] instanceof IAccountPropertyCollection) {
throw new \RuntimeException('Requested collection is not an IAccountPropertyCollection');
if (!array_key_exists($propertyCollectionName, $this->properties)) {
$this->properties[$propertyCollectionName] = new AccountPropertyCollection($propertyCollectionName);
}
return $this->properties[$propertyCollection];
if (!$this->properties[$propertyCollectionName] instanceof IAccountPropertyCollection) {
throw new RuntimeException('Requested collection is not an IAccountPropertyCollection');
}
return $this->properties[$propertyCollectionName];
}
}
251 changes: 123 additions & 128 deletions lib/private/Accounts/AccountManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,10 @@ protected function sanitizeWebsite(IAccountProperty $property, bool $throwOnData
}

protected function updateUser(IUser $user, array $data, bool $throwOnData = false): array {
$userData = $this->getUser($user, false);
$oldUserData = $this->getUser($user, false);
$updated = true;

if ($userData !== $data) {
if ($oldUserData !== $data) {
$this->updateExistingUser($user, $data);
} else {
// nothing needs to be done if new and old data set are the same
Expand Down Expand Up @@ -294,10 +294,8 @@ protected function getUser(IUser $user, bool $insertIfNotExists = true): array {
return $userData;
}

$userDataArray = json_decode($accountData[0]['data'], true);
$jsonError = json_last_error();
if ($userDataArray === null || $userDataArray === [] || $jsonError !== JSON_ERROR_NONE) {
$this->logger->critical("User data of $uid contained invalid JSON (error $jsonError), hence falling back to a default user record");
$userDataArray = $this->importFromJson($accountData[0]['data'], $uid);
if ($userDataArray === null || $userDataArray === []) {
return $this->buildDefaultUserRecord($user);
}

Expand Down Expand Up @@ -346,7 +344,8 @@ protected function checkEmailVerification(IAccount $updatedAccount, array $oldDa
} catch (PropertyDoesNotExistException $e) {
return;
}
if ($oldData[self::PROPERTY_EMAIL]['value'] !== $property->getValue()) {
$oldMail = isset($oldData[self::PROPERTY_EMAIL]) ? $oldData[self::PROPERTY_EMAIL]['value']['value'] : '';
if ($oldMail !== $property->getValue()) {
$this->jobList->add(VerifyUserData::class,
[
'verificationCode' => '',
Expand All @@ -364,25 +363,12 @@ protected function checkEmailVerification(IAccount $updatedAccount, array $oldDa

/**
* make sure that all expected data are set
*
* @param array $userData
* @return array
*/
protected function addMissingDefaultValues(array $userData) {
foreach ($userData as $key => $value) {
if (!isset($userData[$key]['verified']) && !$this->isCollection($key)) {
$userData[$key]['verified'] = self::NOT_VERIFIED;
protected function addMissingDefaultValues(array $userData): array {
foreach ($userData as $i => $value) {
if (!isset($value['verified'])) {
$userData[$i]['verified'] = self::NOT_VERIFIED;
}
if ($this->isCollection($key)) {
foreach ($value as &$singlePropertyData) {
$singlePropertyData['name'] = $key;
}
} else {
$userData[$key]['name'] = $key;
}
}
if (!isset($userData[IAccountManager::COLLECTION_EMAIL])) {
$userData[IAccountManager::COLLECTION_EMAIL] = [];
}

return $userData;
Expand Down Expand Up @@ -415,23 +401,6 @@ protected function updateVerificationStatus(IAccount $updatedAccount, array $old
}
}

protected function dataArrayToJson(array $accountData): string {
$jsonData = [];
foreach ($accountData as $property => $data) {
//$property = $data['name'];
unset($data['name']);
if ($this->isCollection($property)) {
if (!isset($jsonData[$property])) {
$jsonData[$property] = [];
}
$jsonData[$property][] = $data;
} else {
$jsonData[$property] = $data;
}
}
return json_encode($jsonData);
}

/**
* add new user to accounts table
*
Expand All @@ -440,7 +409,7 @@ protected function dataArrayToJson(array $accountData): string {
*/
protected function insertNewUser(IUser $user, array $data): void {
$uid = $user->getUID();
$jsonEncodedData = $this->dataArrayToJson($data);
$jsonEncodedData = $this->prepareJson($data);
$query = $this->connection->getQueryBuilder();
$query->insert($this->table)
->values(
Expand All @@ -455,6 +424,49 @@ protected function insertNewUser(IUser $user, array $data): void {
$this->writeUserData($user, $data);
}

protected function prepareJson(array $data): string {
$preparedData = [];
foreach ($data as $dataRow) {
$propertyName = $dataRow['name'];
unset($dataRow['name']);
if (!$this->isCollection($propertyName)) {
$preparedData[$propertyName] = $dataRow;
continue;
}
if (!isset($preparedData[$propertyName])) {
$preparedData[$propertyName] = [];
}
$preparedData[$propertyName][] = $dataRow;
}
return json_encode($preparedData);
}

protected function importFromJson(string $json, string $userId): ?array {
$result = [];
$jsonArray = json_decode($json, true);
$jsonError = json_last_error();
if ($jsonError !== JSON_ERROR_NONE) {
$this->logger->critical(
'User data of {uid} contained invalid JSON (error {json_error}), hence falling back to a default user record',
[
'uid' => $userId,
'json_error' => $jsonError
]
);
return null;
}
foreach ($jsonArray as $propertyName => $row) {
if (!$this->isCollection($propertyName)) {
$result[] = array_merge($row, ['name' => $propertyName]);
continue;
}
foreach ($row as $singleRow) {
$result[] = array_merge($singleRow, ['name' => $propertyName]);
}
}
return $result;
}

/**
* update existing user in accounts table
*
Expand All @@ -463,12 +475,12 @@ protected function insertNewUser(IUser $user, array $data): void {
*/
protected function updateExistingUser(IUser $user, array $data): void {
$uid = $user->getUID();
$jsonEncodedData = json_encode($data);
$jsonEncodedData = $this->prepareJson($data);
$query = $this->connection->getQueryBuilder();
$query->update($this->table)
->set('data', $query->createNamedParameter($jsonEncodedData))
->where($query->expr()->eq('uid', $query->createNamedParameter($uid)))
->execute();
->executeStatement();

$this->deleteUserData($user);
$this->writeUserData($user, $data);
Expand All @@ -488,18 +500,12 @@ protected function writeUserData(IUser $user, array $data): void {
}

protected function writeUserDataProperties(IQueryBuilder $query, array $data): void {
foreach ($data as $propertyName => $property) {
if (isset($property['name']) && $property['name'] === self::PROPERTY_AVATAR) {
continue;
}
if ($this->isCollection($property['name'] ?? $propertyName) && !isset($property['name'])) {
foreach ($property as $singleProperty) {
$this->writeUserDataProperties($query, [$propertyName => $singleProperty]);
}
foreach ($data as $property) {
if ($property['name'] === self::PROPERTY_AVATAR) {
continue;
}

$query->setParameter('name', $property['name'] ?? $propertyName)
$query->setParameter('name', $property['name'])
->setParameter('value', $property['value'] ?? '');
$query->executeStatement();
}
Expand All @@ -513,79 +519,69 @@ protected function writeUserDataProperties(IQueryBuilder $query, array $data): v
*/
protected function buildDefaultUserRecord(IUser $user) {
return [
self::PROPERTY_DISPLAYNAME =>
[
'name' => self::PROPERTY_DISPLAYNAME,
'value' => $user->getDisplayName(),
'scope' => self::SCOPE_FEDERATED,
'verified' => self::NOT_VERIFIED,
],
self::PROPERTY_ADDRESS =>
[
'name' => self::PROPERTY_ADDRESS,
'value' => '',
'scope' => self::SCOPE_LOCAL,
'verified' => self::NOT_VERIFIED,
],
self::PROPERTY_WEBSITE =>
[
'name' => self::PROPERTY_WEBSITE,
'value' => '',
'scope' => self::SCOPE_LOCAL,
'verified' => self::NOT_VERIFIED,
],
self::PROPERTY_EMAIL =>
[
'name' => self::PROPERTY_EMAIL,
'value' => $user->getEMailAddress(),
'scope' => self::SCOPE_FEDERATED,
'verified' => self::NOT_VERIFIED,
],
self::PROPERTY_AVATAR =>
[
'name' => self::PROPERTY_AVATAR,
'scope' => self::SCOPE_FEDERATED
],
self::PROPERTY_PHONE =>
[
'name' => self::PROPERTY_PHONE,
'value' => '',
'scope' => self::SCOPE_LOCAL,
'verified' => self::NOT_VERIFIED,
],
self::PROPERTY_TWITTER =>
[
'name' => self::PROPERTY_TWITTER,
'value' => '',
'scope' => self::SCOPE_LOCAL,
'verified' => self::NOT_VERIFIED,
],
self::COLLECTION_EMAIL => [],
[
'name' => self::PROPERTY_DISPLAYNAME,
'value' => $user->getDisplayName(),
'scope' => self::SCOPE_FEDERATED,
'verified' => self::NOT_VERIFIED,
],
[
'name' => self::PROPERTY_ADDRESS,
'value' => '',
'scope' => self::SCOPE_LOCAL,
'verified' => self::NOT_VERIFIED,
],
[
'name' => self::PROPERTY_WEBSITE,
'value' => '',
'scope' => self::SCOPE_LOCAL,
'verified' => self::NOT_VERIFIED,
],
[
'name' => self::PROPERTY_EMAIL,
'value' => $user->getEMailAddress(),
'scope' => self::SCOPE_FEDERATED,
'verified' => self::NOT_VERIFIED,
],
[
'name' => self::PROPERTY_AVATAR,
'scope' => self::SCOPE_FEDERATED
],
[
'name' => self::PROPERTY_PHONE,
'value' => '',
'scope' => self::SCOPE_LOCAL,
'verified' => self::NOT_VERIFIED,
],
[
'name' => self::PROPERTY_TWITTER,
'value' => '',
'scope' => self::SCOPE_LOCAL,
'verified' => self::NOT_VERIFIED,
],
];
}

private function arrayDataToCollection(string $collectionName, array $data): IAccountPropertyCollection {
$collection = new AccountPropertyCollection($collectionName);
foreach ($data as $propertyData) {
$p = new AccountProperty(
$collectionName,
$propertyData['value'] ?? '',
$propertyData['scope'] ?? self::SCOPE_LOCAL,
$propertyData['verified'] ?? self::NOT_VERIFIED,
''
);
$collection->addProperty($p);
}
private function arrayDataToCollection(IAccount $account, array $data): IAccountPropertyCollection {
$collection = $account->getPropertyCollection($data['name']);
$p = new AccountProperty(
$data['name'],
$data['value'] ?? '',
$data['scope'] ?? self::SCOPE_LOCAL,
$data['verified'] ?? self::NOT_VERIFIED,
''
);
$collection->addProperty($p);
return $collection;
}

private function parseAccountData(IUser $user, $data): Account {
$account = new Account($user);
foreach ($data as $property => $accountData) {
if ($this->isCollection($property)) {
$account->setPropertyCollection($this->arrayDataToCollection($property, $accountData));
foreach ($data as $accountData) {
if ($this->isCollection($accountData['name'])) {
$account->setPropertyCollection($this->arrayDataToCollection($account, $accountData));
} else {
$account->setProperty($property, $accountData['value'] ?? '', $accountData['scope'] ?? self::SCOPE_LOCAL, $accountData['verified'] ?? self::NOT_VERIFIED);
$account->setProperty($accountData['name'], $accountData['value'] ?? '', $accountData['scope'] ?? self::SCOPE_LOCAL, $accountData['verified'] ?? self::NOT_VERIFIED);
}
}
return $account;
Expand All @@ -596,17 +592,6 @@ public function getAccount(IUser $user): IAccount {
}

public function updateAccount(IAccount $account): void {
$data = [];

foreach ($account->getAllProperties() as $property) {
$data[] = [
'name' => $property->getName(),
'value' => $property->getValue(),
'scope' => $property->getScope(),
'verified' => $property->getVerified(),
];
}

$this->testValueLengths(iterator_to_array($account->getAllProperties()), true);
try {
$property = $account->getProperty(self::PROPERTY_PHONE);
Expand Down Expand Up @@ -639,6 +624,16 @@ public function updateAccount(IAccount $account): void {
$this->updateVerificationStatus($account, $oldData);
$this->checkEmailVerification($account, $oldData);

$data = [];
foreach ($account->getAllProperties() as $property) {
$data[] = [
'name' => $property->getName(),
'value' => $property->getValue(),
'scope' => $property->getScope(),
'verified' => $property->getVerified(),
];
}

$this->updateUser($account->getUser(), $data, true);
}
}
Loading

0 comments on commit 3561025

Please sign in to comment.