Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(db): switch from settype to casts #48010

Merged
merged 1 commit into from
Sep 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions lib/private/Tagging/Tag.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,16 @@ public function __construct($owner = null, $type = null, $name = null) {
* @todo migrate existing database columns to the correct names
* to be able to drop this direct mapping
*/
public function columnToProperty($columnName) {
public function columnToProperty(string $columnName): string {
if ($columnName === 'category') {
return 'name';
} elseif ($columnName === 'uid') {
}

if ($columnName === 'uid') {
return 'owner';
} else {
return parent::columnToProperty($columnName);
}

return parent::columnToProperty($columnName);
}

/**
Expand All @@ -61,13 +63,15 @@ public function columnToProperty($columnName) {
* @param string $property the name of the property
* @return string the column name
*/
public function propertyToColumn($property) {
public function propertyToColumn(string $property): string {
if ($property === 'name') {
return 'category';
} elseif ($property === 'owner') {
}

if ($property === 'owner') {
return 'uid';
} else {
return parent::propertyToColumn($property);
}

return parent::propertyToColumn($property);
}
}
109 changes: 63 additions & 46 deletions lib/public/AppFramework/Db/Entity.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
/**
* @method int getId()
* @method void setId(int $id)
* @psalm-type AllowedTypes = 'json'|'blob'|'datetime'|'string'|'int'|'integer'|'bool'|'boolean'|'float'|'double'|'array'|'object'
* @since 7.0.0
* @psalm-consistent-constructor
*/
Expand All @@ -23,6 +24,7 @@ abstract class Entity {
public $id;

private array $_updatedFields = [];
/** @var array<string, AllowedTypes> */
private array $_fieldTypes = ['id' => 'integer'];

/**
Expand Down Expand Up @@ -64,10 +66,10 @@ public static function fromRow(array $row): static {


/**
* @return array with attribute and type
* @return array<string, AllowedTypes> with attribute and type
* @since 7.0.0
*/
public function getFieldTypes() {
public function getFieldTypes(): array {
return $this->_fieldTypes;
}

Expand All @@ -76,50 +78,62 @@ public function getFieldTypes() {
* Marks the entity as clean needed for setting the id after the insertion
* @since 7.0.0
*/
public function resetUpdatedFields() {
public function resetUpdatedFields(): void {
$this->_updatedFields = [];
}

/**
* Generic setter for properties
*
* @throws \InvalidArgumentException
* @since 7.0.0
*
*/
protected function setter(string $name, array $args): void {
// setters should only work for existing attributes
if (property_exists($this, $name)) {
if ($args[0] === $this->$name) {
return;
}
$this->markFieldUpdated($name);

// if type definition exists, cast to correct type
if ($args[0] !== null && array_key_exists($name, $this->_fieldTypes)) {
$type = $this->_fieldTypes[$name];
if ($type === 'blob') {
// (B)LOB is treated as string when we read from the DB
if (is_resource($args[0])) {
$args[0] = stream_get_contents($args[0]);
}
$type = 'string';
if (!property_exists($this, $name)) {
throw new \BadFunctionCallException($name . ' is not a valid attribute');
}

if ($args[0] === $this->$name) {
return;
}
$this->markFieldUpdated($name);

// if type definition exists, cast to correct type
if ($args[0] !== null && array_key_exists($name, $this->_fieldTypes)) {
$type = $this->_fieldTypes[$name];
if ($type === 'blob') {
// (B)LOB is treated as string when we read from the DB
if (is_resource($args[0])) {
$args[0] = stream_get_contents($args[0]);
}
$type = 'string';
}

if ($type === 'datetime') {
if (!$args[0] instanceof \DateTime) {
$args[0] = new \DateTime($args[0]);
}
} elseif ($type === 'json') {
if (!is_array($args[0])) {
$args[0] = json_decode($args[0], true);
}
} else {
settype($args[0], $type);
if ($type === 'datetime') {
if (!$args[0] instanceof \DateTime) {
$args[0] = new \DateTime($args[0]);
}
} elseif ($type === 'json') {
if (!is_array($args[0])) {
$args[0] = json_decode($args[0], true);
}
} else {
$args[0] = match($type) {
'string' => (string)$args[0],
'bool', 'boolean', => (bool)$args[0],
'int', 'integer', => (int)$args[0],
'float' => (float)$args[0],
'double' => (float)$args[0],
'array' => (array)$args[0],
'object' => (object)$args[0],
default => new \InvalidArgumentException()
};
}
$this->$name = $args[0];
} else {
throw new \BadFunctionCallException($name .
' is not a valid attribute');
}
$this->$name = $args[0];

}

/**
Expand Down Expand Up @@ -182,16 +196,17 @@ protected function markFieldUpdated(string $attribute): void {

/**
* Transform a database columnname to a property
*
* @param string $columnName the name of the column
* @return string the property name
* @since 7.0.0
*/
public function columnToProperty($columnName) {
public function columnToProperty(string $columnName): string {
$parts = explode('_', $columnName);
$property = null;
$property = '';

foreach ($parts as $part) {
if ($property === null) {
if ($property === '') {
$property = $part;
} else {
$property .= ucfirst($part);
Expand All @@ -204,16 +219,17 @@ public function columnToProperty($columnName) {

/**
* Transform a property to a database column name
*
* @param string $property the name of the property
* @return string the column name
* @since 7.0.0
*/
public function propertyToColumn($property) {
public function propertyToColumn(string $property): string {
$parts = preg_split('/(?=[A-Z])/', $property);
$column = null;

$column = '';
foreach ($parts as $part) {
if ($column === null) {
if ($column === '') {
$column = $part;
} else {
$column .= '_' . lcfirst($part);
Expand All @@ -228,32 +244,34 @@ public function propertyToColumn($property) {
* @return array array of updated fields for update query
* @since 7.0.0
*/
public function getUpdatedFields() {
public function getUpdatedFields(): array {
return $this->_updatedFields;
}


/**
* Adds type information for a field so that its automatically casted to
* Adds type information for a field so that it's automatically cast to
* that value once its being returned from the database
*
* @param string $fieldName the name of the attribute
* @param string $type the type which will be used to call settype()
* @param AllowedTypes $type the type which will be used to match a cast
* @since 7.0.0
*/
protected function addType($fieldName, $type) {
protected function addType(string $fieldName, string $type): void {
$this->_fieldTypes[$fieldName] = $type;
}


/**
* Slugify the value of a given attribute
* Warning: This doesn't result in a unique value
*
* @param string $attributeName the name of the attribute, which value should be slugified
* @return string slugified value
* @since 7.0.0
* @deprecated 24.0.0
*/
public function slugify($attributeName) {
public function slugify(string $attributeName): string {
// toSlug should only work for existing attributes
if (property_exists($this, $attributeName)) {
$value = $this->$attributeName;
Expand All @@ -262,9 +280,8 @@ public function slugify($attributeName) {
$value = strtolower($value);
// trim '-'
return trim($value, '-');
} else {
throw new \BadFunctionCallException($attributeName .
' is not a valid attribute');
}

throw new \BadFunctionCallException($attributeName . ' is not a valid attribute');
}
}
8 changes: 4 additions & 4 deletions tests/lib/AppTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -460,9 +460,9 @@ public function appConfigValuesProvider() {
public function testEnabledApps($user, $expectedApps, $forceAll) {
$userManager = \OC::$server->getUserManager();
$groupManager = \OC::$server->getGroupManager();
$user1 = $userManager->createUser(self::TEST_USER1, self::TEST_USER1);
$user2 = $userManager->createUser(self::TEST_USER2, self::TEST_USER2);
$user3 = $userManager->createUser(self::TEST_USER3, self::TEST_USER3);
$user1 = $userManager->createUser(self::TEST_USER1, 'NotAnEasyPassword123456+');
$user2 = $userManager->createUser(self::TEST_USER2, 'NotAnEasyPassword123456_');
$user3 = $userManager->createUser(self::TEST_USER3, 'NotAnEasyPassword123456?');

$group1 = $groupManager->createGroup(self::TEST_GROUP1);
$group1->addUser($user1);
Expand Down Expand Up @@ -508,7 +508,7 @@ public function testEnabledApps($user, $expectedApps, $forceAll) {
*/
public function testEnabledAppsCache() {
$userManager = \OC::$server->getUserManager();
$user1 = $userManager->createUser(self::TEST_USER1, self::TEST_USER1);
$user1 = $userManager->createUser(self::TEST_USER1, 'NotAnEasyPassword123456+');

\OC_User::setUserId(self::TEST_USER1);

Expand Down
2 changes: 1 addition & 1 deletion tests/lib/Comments/ManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ public function testDeleteReferencesOfActor() {
}

public function testDeleteReferencesOfActorWithUserManagement() {
$user = \OC::$server->getUserManager()->createUser('xenia', '123456');
$user = \OC::$server->getUserManager()->createUser('xenia', 'NotAnEasyPassword123456+');
$this->assertTrue($user instanceof IUser);

$manager = \OC::$server->get(ICommentsManager::class);
Expand Down
2 changes: 1 addition & 1 deletion tests/lib/Files/Cache/UpdaterLegacyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ protected function setUp(): void {
self::$user = $this->getUniqueID();
}

\OC::$server->getUserManager()->createUser(self::$user, 'password');
\OC::$server->getUserManager()->createUser(self::$user, 'NotAnEasyPassword123456+');
$this->loginAsUser(self::$user);

Filesystem::init(self::$user, '/' . self::$user . '/files');
Expand Down
Loading