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.

Sign up for GitHub

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

Jump to bottom

fix(entity): Fix mapping of old/sub-types to actually supported datab… #48837

Merged
merged 1 commit into from
Oct 23, 2024
+61 −5
Merged

fix(entity): Fix mapping of old/sub-types to actually supported datab… #48837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 22 additions & 0 deletions lib/public/AppFramework/Db/Entity.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,15 @@ protected function setter(string $name, array $args): void {
}

switch ($type) {
case Types::BIGINT:
case Types::SMALLINT:
settype($args[0], Types::INTEGER);
break;
case Types::BINARY:
case Types::DECIMAL:
case Types::TEXT:
settype($args[0], Types::STRING);
break;
case Types::TIME:
case Types::DATE:
case Types::DATETIME:
Expand Down Expand Up @@ -260,9 +269,22 @@ public function getUpdatedFields(): array {
*
* @param string $fieldName the name of the attribute
* @param \OCP\DB\Types::* $type the type which will be used to match a cast
* @since 31.0.0 Parameter $type is now restricted to {@see \OCP\DB\Types} constants. The formerly accidentally supported types 'int'|'bool'|'double' are mapped to Types::INTEGER|Types::BOOLEAN|Types::FLOAT accordingly.
* @since 7.0.0
*/
protected function addType(string $fieldName, string $type): void {
/** @psalm-suppress TypeDoesNotContainType */
if (in_array($type, ['bool', 'double', 'int', 'array', 'object'], true)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we document those to satisfy psalm? Or yolo?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the options are this, right?

  1. Document and make it somewhat official by doing so
  2. yolo and apps have to baseline or use the types from the constants

I would prefer to fix the apps (so 2), but not sure if this is too much effort and I do not want to make all devs angry...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a @since entry to the method to explain it, together with a psalm-suppress.
We could backport the setter() change, to make sure using Types::TEXT and the others works also on stable branches. But then the apps would have to depend on a specific patch release or still need a version switch.

So for now I would say they will have to use the psalm baseline until they dropped support for everything < 31 and then move to Types?

// Mapping legacy strings to the actual types
$type = match ($type) {
'int' => Types::INTEGER,
nickvergessen marked this conversation as resolved.
Show resolved Hide resolved
'bool' => Types::BOOLEAN,
'double' => Types::FLOAT,
'array',
'object' => Types::STRING,
};
}

$this->_fieldTypes[$fieldName] = $type;
}

Expand Down
44 changes: 39 additions & 5 deletions tests/lib/AppFramework/Db/EntityTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,31 @@ class TestEntity extends Entity {
protected $name;
protected $email;
protected $testId;
protected $smallInt;
protected $bigInt;
protected $preName;
protected $trueOrFalse;
protected $anotherBool;
protected $text;
protected $longText;
protected $time;
protected $datetime;

public function __construct($name = null) {
$this->addType('testId', Types::INTEGER);
$this->addType('trueOrFalse', 'bool');
$this->addType('smallInt', Types::SMALLINT);
$this->addType('bigInt', Types::BIGINT);
$this->addType('anotherBool', Types::BOOLEAN);
$this->addType('text', Types::TEXT);
$this->addType('longText', Types::BLOB);
$this->addType('time', Types::TIME);
$this->addType('datetime', Types::DATETIME_IMMUTABLE);

// Legacy types
$this->addType('trueOrFalse', 'bool');
$this->addType('legacyInt', 'int');
$this->addType('doubleNowFloat', 'double');

$this->name = $name;
}

Expand Down Expand Up @@ -200,10 +211,28 @@ public function testSlugify(): void {
}


public function testSetterCasts(): void {
public function dataSetterCasts(): array {
return [
['Id', '3', 3],
['smallInt', '3', 3],
['bigInt', '' . PHP_INT_MAX, PHP_INT_MAX],
['trueOrFalse', 0, false],
['trueOrFalse', 1, true],
['anotherBool', 0, false],
['anotherBool', 1, true],
['text', 33, '33'],
['longText', PHP_INT_MAX, '' . PHP_INT_MAX],
];
}


/**
* @dataProvider dataSetterCasts
*/
public function testSetterCasts(string $field, mixed $in, mixed $out): void {
$entity = new TestEntity();
$entity->setId('3');
$this->assertSame(3, $entity->getId());
$entity->{'set' . $field}($in);
$this->assertSame($out, $entity->{'get' . $field}());
}


Expand Down Expand Up @@ -248,11 +277,16 @@ public function testGetFieldTypes(): void {
$this->assertEquals([
'id' => Types::INTEGER,
'testId' => Types::INTEGER,
'trueOrFalse' => 'bool',
'smallInt' => Types::SMALLINT,
'bigInt' => Types::BIGINT,
'anotherBool' => Types::BOOLEAN,
'text' => Types::TEXT,
'longText' => Types::BLOB,
'time' => Types::TIME,
'datetime' => Types::DATETIME_IMMUTABLE,
'trueOrFalse' => Types::BOOLEAN,
'legacyInt' => Types::INTEGER,
'doubleNowFloat' => Types::FLOAT,
], $entity->getFieldTypes());
}

Expand Down
Loading