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(AppFramework): Add full support for date / time / datetime columns #47329

Merged
merged 4 commits into from
Oct 18, 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
13 changes: 7 additions & 6 deletions apps/contactsinteraction/lib/Db/RecentContact.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace OCA\ContactsInteraction\Db;

use OCP\AppFramework\Db\Entity;
use OCP\DB\Types;

/**
* @method void setActorUid(string $uid)
Expand All @@ -33,11 +34,11 @@ class RecentContact extends Entity {
protected int $lastContact = -1;

public function __construct() {
$this->addType('actorUid', 'string');
$this->addType('uid', 'string');
$this->addType('email', 'string');
$this->addType('federatedCloudId', 'string');
$this->addType('card', 'blob');
$this->addType('lastContact', 'int');
$this->addType('actorUid', Types::STRING);
$this->addType('uid', Types::STRING);
$this->addType('email', Types::STRING);
$this->addType('federatedCloudId', Types::STRING);
$this->addType('card', Types::BLOB);
$this->addType('lastContact', Types::INTEGER);
}
}
7 changes: 4 additions & 3 deletions apps/dav/lib/CalDAV/Proxy/Proxy.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace OCA\DAV\CalDAV\Proxy;

use OCP\AppFramework\Db\Entity;
use OCP\DB\Types;

/**
* @method string getOwnerId()
Expand All @@ -28,8 +29,8 @@ class Proxy extends Entity {
protected $permissions;

public function __construct() {
$this->addType('ownerId', 'string');
$this->addType('proxyId', 'string');
$this->addType('permissions', 'int');
$this->addType('ownerId', Types::STRING);
$this->addType('proxyId', Types::STRING);
$this->addType('permissions', Types::INTEGER);
}
}
9 changes: 5 additions & 4 deletions apps/dav/lib/Db/Direct.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace OCA\DAV\Db;

use OCP\AppFramework\Db\Entity;
use OCP\DB\Types;

/**
* @method string getUserId()
Expand All @@ -34,9 +35,9 @@ class Direct extends Entity {
protected $expiration;

public function __construct() {
$this->addType('userId', 'string');
$this->addType('fileId', 'int');
$this->addType('token', 'string');
$this->addType('expiration', 'int');
$this->addType('userId', Types::STRING);
$this->addType('fileId', Types::INTEGER);
$this->addType('token', Types::STRING);
$this->addType('expiration', Types::INTEGER);
}
}
4 changes: 2 additions & 2 deletions apps/federatedfilesharing/lib/FederatedShareProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ui
->setValue('uid_owner', $qb->createNamedParameter($uidOwner))
->setValue('uid_initiator', $qb->createNamedParameter($sharedBy))
->setValue('permissions', $qb->createNamedParameter($permissions))
->setValue('expiration', $qb->createNamedParameter($expirationDate, IQueryBuilder::PARAM_DATE))
->setValue('expiration', $qb->createNamedParameter($expirationDate, IQueryBuilder::PARAM_DATETIME_MUTABLE))
->setValue('token', $qb->createNamedParameter($token))
->setValue('stime', $qb->createNamedParameter(time()));

Expand Down Expand Up @@ -333,7 +333,7 @@ public function update(IShare $share) {
->set('permissions', $qb->createNamedParameter($share->getPermissions()))
->set('uid_owner', $qb->createNamedParameter($share->getShareOwner()))
->set('uid_initiator', $qb->createNamedParameter($share->getSharedBy()))
->set('expiration', $qb->createNamedParameter($share->getExpirationDate(), IQueryBuilder::PARAM_DATE))
->set('expiration', $qb->createNamedParameter($share->getExpirationDate(), IQueryBuilder::PARAM_DATETIME_MUTABLE))
->executeStatement();

// send the updated permission to the owner/initiator, if they are not the same
Expand Down
2 changes: 1 addition & 1 deletion apps/files_reminders/lib/Db/ReminderMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public function findNotified(DateTime $buffer, ?int $limit = null) {
$qb->select('id', 'user_id', 'file_id', 'due_date', 'updated_at', 'created_at', 'notified')
->from($this->getTableName())
->where($qb->expr()->eq('notified', $qb->createNamedParameter(true, IQueryBuilder::PARAM_BOOL)))
->andWhere($qb->expr()->lt('due_date', $qb->createNamedParameter($buffer, IQueryBuilder::PARAM_DATE)))
->andWhere($qb->expr()->lt('due_date', $qb->createNamedParameter($buffer, IQueryBuilder::PARAM_DATETIME_MUTABLE)))
->orderBy('due_date', 'ASC')
->setMaxResults($limit);

Expand Down
11 changes: 6 additions & 5 deletions apps/oauth2/lib/Db/AccessToken.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
namespace OCA\OAuth2\Db;

use OCP\AppFramework\Db\Entity;
use OCP\DB\Types;

/**
* @method int getTokenId()
Expand Down Expand Up @@ -36,12 +37,12 @@ class AccessToken extends Entity {
protected $tokenCount;

public function __construct() {
$this->addType('id', 'int');
$this->addType('tokenId', 'int');
$this->addType('clientId', 'int');
$this->addType('id', Types::INTEGER);
$this->addType('tokenId', Types::INTEGER);
$this->addType('clientId', Types::INTEGER);
$this->addType('hashedCode', 'string');
$this->addType('encryptedToken', 'string');
$this->addType('codeCreatedAt', 'int');
$this->addType('tokenCount', 'int');
$this->addType('codeCreatedAt', Types::INTEGER);
$this->addType('tokenCount', Types::INTEGER);
Comment on lines -39 to +46
Copy link
Member

Choose a reason for hiding this comment

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

Someone should write a Rector rule for this

Copy link
Member

Choose a reason for hiding this comment

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

Also this is not a clean replacement. Types::INTEGER is 'integer' not 'int' which was used in almost all apps (at least the ones I maintain which now have dozens of Psalm errors 🙈

Copy link
Member

Choose a reason for hiding this comment

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

Came back to check if this was intentional.

@susnux would there be any downside of allow "int" again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Came back to check if this was intentional.

Yes it was, as int, double, and bool were never documented, only integer, float and boolean were part of our documentation (since Nextcloud 20: https://docs.nextcloud.com/server/20/developer_manual/basics/storage/database.html#types ).
So the idea was to still allow them (it does not break) but make usage of it an error for static code checking to make developers aware they are using something undocumented.

Meaning to make it easier for us to change types if needed as we only need to change a single source of truth (the DB types).

would there be any downside of allow "int" again?

From my side: just duplicates a state, but lets add it back to make it less noisy for developers!

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, int indeed wasn't documented!

Copy link
Member

Choose a reason for hiding this comment

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

addType('[^']+', 'int')

Results: 116

addType('[^']+', 'integer')

Results: 175


So while it was never documented, it was working and is used almost 40% of the time 🙈 So yeah we can not have it in docs going forward, but should keep an eye on keeping it working, so apps don't break unnecessarily.

For bool its more based with 17 (bool) to 68 (boolean), but still 20%

double I didn't find a single result (in apps we marketed, ship, support or maintain), neither for array or object.

Copy link
Member

Choose a reason for hiding this comment

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

Ref #48837

}
}
3 changes: 2 additions & 1 deletion apps/oauth2/lib/Db/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
namespace OCA\OAuth2\Db;

use OCP\AppFramework\Db\Entity;
use OCP\DB\Types;

/**
* @method string getClientIdentifier()
Expand All @@ -28,7 +29,7 @@ class Client extends Entity {
protected $secret;

public function __construct() {
$this->addType('id', 'int');
$this->addType('id', Types::INTEGER);
$this->addType('name', 'string');
$this->addType('redirectUri', 'string');
$this->addType('clientIdentifier', 'string');
Expand Down
8 changes: 4 additions & 4 deletions apps/sharebymail/lib/ShareByMailProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,7 @@ protected function addShareToDB(
->setValue('permissions', $qb->createNamedParameter($permissions))
->setValue('token', $qb->createNamedParameter($token))
->setValue('password', $qb->createNamedParameter($password))
->setValue('password_expiration_time', $qb->createNamedParameter($passwordExpirationTime, IQueryBuilder::PARAM_DATE))
->setValue('password_expiration_time', $qb->createNamedParameter($passwordExpirationTime, IQueryBuilder::PARAM_DATETIME_MUTABLE))
->setValue('password_by_talk', $qb->createNamedParameter($sendPasswordByTalk, IQueryBuilder::PARAM_BOOL))
->setValue('stime', $qb->createNamedParameter(time()))
->setValue('hide_download', $qb->createNamedParameter((int)$hideDownload, IQueryBuilder::PARAM_INT))
Expand All @@ -712,7 +712,7 @@ protected function addShareToDB(

$qb->setValue('attributes', $qb->createNamedParameter($shareAttributes));
if ($expirationTime !== null) {
$qb->setValue('expiration', $qb->createNamedParameter($expirationTime, IQueryBuilder::PARAM_DATE));
$qb->setValue('expiration', $qb->createNamedParameter($expirationTime, IQueryBuilder::PARAM_DATETIME_MUTABLE));
}

$qb->executeStatement();
Expand Down Expand Up @@ -752,10 +752,10 @@ public function update(IShare $share, ?string $plainTextPassword = null): IShare
->set('uid_owner', $qb->createNamedParameter($share->getShareOwner()))
->set('uid_initiator', $qb->createNamedParameter($share->getSharedBy()))
->set('password', $qb->createNamedParameter($share->getPassword()))
->set('password_expiration_time', $qb->createNamedParameter($share->getPasswordExpirationTime(), IQueryBuilder::PARAM_DATE))
->set('password_expiration_time', $qb->createNamedParameter($share->getPasswordExpirationTime(), IQueryBuilder::PARAM_DATETIME_MUTABLE))
->set('label', $qb->createNamedParameter($share->getLabel()))
->set('password_by_talk', $qb->createNamedParameter($share->getSendPasswordByTalk(), IQueryBuilder::PARAM_BOOL))
->set('expiration', $qb->createNamedParameter($share->getExpirationDate(), IQueryBuilder::PARAM_DATE))
->set('expiration', $qb->createNamedParameter($share->getExpirationDate(), IQueryBuilder::PARAM_DATETIME_MUTABLE))
->set('note', $qb->createNamedParameter($share->getNote()))
->set('hide_download', $qb->createNamedParameter((int)$share->getHideDownload(), IQueryBuilder::PARAM_INT))
->set('attributes', $qb->createNamedParameter($shareAttributes))
Expand Down
7 changes: 4 additions & 3 deletions apps/user_status/lib/Db/UserStatus.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace OCA\UserStatus\Db;

use OCP\AppFramework\Db\Entity;
use OCP\DB\Types;

/**
* Class UserStatus
Expand Down Expand Up @@ -73,13 +74,13 @@ class UserStatus extends Entity {
public function __construct() {
$this->addType('userId', 'string');
$this->addType('status', 'string');
$this->addType('statusTimestamp', 'int');
$this->addType('statusTimestamp', Types::INTEGER);
$this->addType('isUserDefined', 'boolean');
$this->addType('messageId', 'string');
$this->addType('customIcon', 'string');
$this->addType('customMessage', 'string');
$this->addType('clearAt', 'int');
$this->addType('clearAt', Types::INTEGER);
$this->addType('isBackup', 'boolean');
$this->addType('statusMessageTimestamp', 'int');
$this->addType('statusMessageTimestamp', Types::INTEGER);
}
}
5 changes: 3 additions & 2 deletions core/Db/LoginFlowV2.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace OC\Core\Db;

use OCP\AppFramework\Db\Entity;
use OCP\DB\Types;

/**
* @method int getTimestamp()
Expand Down Expand Up @@ -55,8 +56,8 @@ class LoginFlowV2 extends Entity {
protected $appPassword;

public function __construct() {
$this->addType('timestamp', 'int');
$this->addType('started', 'int');
$this->addType('timestamp', Types::INTEGER);
$this->addType('started', Types::INTEGER);
$this->addType('pollToken', 'string');
$this->addType('loginToken', 'string');
$this->addType('publicKey', 'string');
Expand Down
15 changes: 8 additions & 7 deletions lib/private/Authentication/Token/PublicKeyToken.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use OCP\AppFramework\Db\Entity;
use OCP\Authentication\Token\IToken;
use OCP\DB\Types;

/**
* @method void setId(int $id)
Expand Down Expand Up @@ -88,16 +89,16 @@ public function __construct() {
$this->addType('passwordHash', 'string');
$this->addType('name', 'string');
$this->addType('token', 'string');
$this->addType('type', 'int');
$this->addType('remember', 'int');
$this->addType('lastActivity', 'int');
$this->addType('lastCheck', 'int');
$this->addType('type', Types::INTEGER);
$this->addType('remember', Types::INTEGER);
$this->addType('lastActivity', Types::INTEGER);
$this->addType('lastCheck', Types::INTEGER);
$this->addType('scope', 'string');
$this->addType('expires', 'int');
$this->addType('expires', Types::INTEGER);
$this->addType('publicKey', 'string');
$this->addType('privateKey', 'string');
$this->addType('version', 'int');
$this->addType('passwordInvalid', 'bool');
$this->addType('version', Types::INTEGER);
$this->addType('passwordInvalid', Types::BOOLEAN);
}

public function getId(): int {
Expand Down
20 changes: 10 additions & 10 deletions lib/private/Comments/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -440,14 +440,14 @@ public function getCommentsWithVerbForObjectSinceComment(
$query->expr()->orX(
$query->expr()->lt(
'creation_timestamp',
$query->createNamedParameter($lastKnownCommentDateTime, IQueryBuilder::PARAM_DATE),
IQueryBuilder::PARAM_DATE
$query->createNamedParameter($lastKnownCommentDateTime, IQueryBuilder::PARAM_DATETIME_MUTABLE),
IQueryBuilder::PARAM_DATETIME_MUTABLE
),
$query->expr()->andX(
$query->expr()->eq(
'creation_timestamp',
$query->createNamedParameter($lastKnownCommentDateTime, IQueryBuilder::PARAM_DATE),
IQueryBuilder::PARAM_DATE
$query->createNamedParameter($lastKnownCommentDateTime, IQueryBuilder::PARAM_DATETIME_MUTABLE),
IQueryBuilder::PARAM_DATETIME_MUTABLE
),
$idComparison
)
Expand All @@ -463,14 +463,14 @@ public function getCommentsWithVerbForObjectSinceComment(
$query->expr()->orX(
$query->expr()->gt(
'creation_timestamp',
$query->createNamedParameter($lastKnownCommentDateTime, IQueryBuilder::PARAM_DATE),
IQueryBuilder::PARAM_DATE
$query->createNamedParameter($lastKnownCommentDateTime, IQueryBuilder::PARAM_DATETIME_MUTABLE),
IQueryBuilder::PARAM_DATETIME_MUTABLE
),
$query->expr()->andX(
$query->expr()->eq(
'creation_timestamp',
$query->createNamedParameter($lastKnownCommentDateTime, IQueryBuilder::PARAM_DATE),
IQueryBuilder::PARAM_DATE
$query->createNamedParameter($lastKnownCommentDateTime, IQueryBuilder::PARAM_DATETIME_MUTABLE),
IQueryBuilder::PARAM_DATETIME_MUTABLE
),
$idComparison
)
Expand Down Expand Up @@ -740,7 +740,7 @@ public function getLastCommentBeforeDate(string $objectType, string $objectId, \
->from('comments')
->where($query->expr()->eq('object_type', $query->createNamedParameter($objectType)))
->andWhere($query->expr()->eq('object_id', $query->createNamedParameter($objectId)))
->andWhere($query->expr()->lt('creation_timestamp', $query->createNamedParameter($beforeDate, IQueryBuilder::PARAM_DATE)))
->andWhere($query->expr()->lt('creation_timestamp', $query->createNamedParameter($beforeDate, IQueryBuilder::PARAM_DATETIME_MUTABLE)))
->orderBy('creation_timestamp', 'desc');

if ($verb !== '') {
Expand Down Expand Up @@ -1551,7 +1551,7 @@ public function deleteCommentsExpiredAtObject(string $objectType, string $object
$qb = $this->dbConn->getQueryBuilder();
$qb->delete('comments')
->where($qb->expr()->lte('expire_date',
$qb->createNamedParameter($this->timeFactory->getDateTime(), IQueryBuilder::PARAM_DATE)))
$qb->createNamedParameter($this->timeFactory->getDateTime(), IQueryBuilder::PARAM_DATETIME_MUTABLE)))
->andWhere($qb->expr()->eq('object_type', $qb->createNamedParameter($objectType)));

if ($objectId !== '') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ public function iLike($x, $y, $type = null): string {
* @return array|IQueryFunction|string
*/
protected function prepareColumn($column, $type) {
if ($type === IQueryBuilder::PARAM_DATE && !is_array($column) && !($column instanceof IParameter) && !($column instanceof ILiteral)) {
if ($type !== null
&& !is_array($column)
&& !($column instanceof IParameter)
&& !($column instanceof ILiteral)
&& (str_starts_with($type, 'date') || str_starts_with($type, 'time'))) {
return $this->castColumn($column, $type);
}

Expand All @@ -44,9 +48,21 @@ protected function prepareColumn($column, $type) {
* @return IQueryFunction
*/
public function castColumn($column, $type): IQueryFunction {
if ($type === IQueryBuilder::PARAM_DATE) {
$column = $this->helper->quoteColumnName($column);
return new QueryFunction('DATETIME(' . $column . ')');
switch ($type) {
case IQueryBuilder::PARAM_DATE_MUTABLE:
case IQueryBuilder::PARAM_DATE_IMMUTABLE:
$column = $this->helper->quoteColumnName($column);
return new QueryFunction('DATE(' . $column . ')');
case IQueryBuilder::PARAM_DATETIME_MUTABLE:
case IQueryBuilder::PARAM_DATETIME_IMMUTABLE:
case IQueryBuilder::PARAM_DATETIME_TZ_MUTABLE:
case IQueryBuilder::PARAM_DATETIME_TZ_IMMUTABLE:
$column = $this->helper->quoteColumnName($column);
return new QueryFunction('DATETIME(' . $column . ')');
case IQueryBuilder::PARAM_TIME_MUTABLE:
case IQueryBuilder::PARAM_TIME_IMMUTABLE:
$column = $this->helper->quoteColumnName($column);
return new QueryFunction('TIME(' . $column . ')');
}

return parent::castColumn($column, $type);
Expand Down
4 changes: 2 additions & 2 deletions lib/private/Security/RateLimiting/Backend/DatabaseBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ private function getExistingAttemptCount(
$qb = $this->dbConnection->getQueryBuilder();
$qb->delete(self::TABLE_NAME)
->where(
$qb->expr()->lte('delete_after', $qb->createNamedParameter($currentTime, IQueryBuilder::PARAM_DATE))
$qb->expr()->lte('delete_after', $qb->createNamedParameter($currentTime, IQueryBuilder::PARAM_DATETIME_MUTABLE))
)
->executeStatement();

Expand Down Expand Up @@ -87,7 +87,7 @@ public function registerAttempt(
$qb->insert(self::TABLE_NAME)
->values([
'hash' => $qb->createNamedParameter($identifier, IQueryBuilder::PARAM_STR),
'delete_after' => $qb->createNamedParameter($deleteAfter, IQueryBuilder::PARAM_DATE),
'delete_after' => $qb->createNamedParameter($deleteAfter, IQueryBuilder::PARAM_DATETIME_MUTABLE),
]);

if (!$this->config->getSystemValueBool('ratelimit.protection.enabled', true)) {
Expand Down
8 changes: 4 additions & 4 deletions lib/private/Share20/DefaultShareProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ public function update(\OCP\Share\IShare $share) {
->set('attributes', $qb->createNamedParameter($shareAttributes))
->set('item_source', $qb->createNamedParameter($share->getNode()->getId()))
->set('file_source', $qb->createNamedParameter($share->getNode()->getId()))
->set('expiration', $qb->createNamedParameter($expirationDate, IQueryBuilder::PARAM_DATE))
->set('expiration', $qb->createNamedParameter($expirationDate, IQueryBuilder::PARAM_DATETIME_MUTABLE))
->set('note', $qb->createNamedParameter($share->getNote()))
->set('accepted', $qb->createNamedParameter($share->getStatus()))
->set('reminder_sent', $qb->createNamedParameter($share->getReminderSent(), IQueryBuilder::PARAM_BOOL))
Expand All @@ -237,7 +237,7 @@ public function update(\OCP\Share\IShare $share) {
->set('attributes', $qb->createNamedParameter($shareAttributes))
->set('item_source', $qb->createNamedParameter($share->getNode()->getId()))
->set('file_source', $qb->createNamedParameter($share->getNode()->getId()))
->set('expiration', $qb->createNamedParameter($expirationDate, IQueryBuilder::PARAM_DATE))
->set('expiration', $qb->createNamedParameter($expirationDate, IQueryBuilder::PARAM_DATETIME_MUTABLE))
->set('note', $qb->createNamedParameter($share->getNote()))
->execute();

Expand All @@ -252,7 +252,7 @@ public function update(\OCP\Share\IShare $share) {
->set('uid_initiator', $qb->createNamedParameter($share->getSharedBy()))
->set('item_source', $qb->createNamedParameter($share->getNode()->getId()))
->set('file_source', $qb->createNamedParameter($share->getNode()->getId()))
->set('expiration', $qb->createNamedParameter($expirationDate, IQueryBuilder::PARAM_DATE))
->set('expiration', $qb->createNamedParameter($expirationDate, IQueryBuilder::PARAM_DATETIME_MUTABLE))
->set('note', $qb->createNamedParameter($share->getNote()))
->execute();

Expand All @@ -279,7 +279,7 @@ public function update(\OCP\Share\IShare $share) {
->set('item_source', $qb->createNamedParameter($share->getNode()->getId()))
->set('file_source', $qb->createNamedParameter($share->getNode()->getId()))
->set('token', $qb->createNamedParameter($share->getToken()))
->set('expiration', $qb->createNamedParameter($expirationDate, IQueryBuilder::PARAM_DATE))
->set('expiration', $qb->createNamedParameter($expirationDate, IQueryBuilder::PARAM_DATETIME_MUTABLE))
->set('note', $qb->createNamedParameter($share->getNote()))
->set('label', $qb->createNamedParameter($share->getLabel()))
->set('hide_download', $qb->createNamedParameter($share->getHideDownload() ? 1 : 0), IQueryBuilder::PARAM_INT)
Expand Down
Loading
Loading