From 9b52663a81ef4db0aa458ed6b34dfaab872e3243 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 20 Jun 2022 17:53:20 +0200 Subject: [PATCH 1/5] Revert "make expression build return IQueryFunction instead of string" This reverts commit 813b50ed428a8bc36817d19c84444e96dbe3b668. Signed-off-by: Robin Appelman --- .../ExpressionBuilder/ExpressionBuilder.php | 96 +++++++++---------- .../MySqlExpressionBuilder.php | 4 +- .../OCIExpressionBuilder.php | 50 +++++----- .../PgSqlExpressionBuilder.php | 4 +- .../SqliteExpressionBuilder.php | 11 +-- lib/private/DB/QueryBuilder/QueryBuilder.php | 12 +-- .../DB/QueryBuilder/IExpressionBuilder.php | 64 ++++++------- lib/public/DB/QueryBuilder/IQueryBuilder.php | 8 +- 8 files changed, 121 insertions(+), 128 deletions(-) diff --git a/lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php b/lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php index 333984bde71f4..ae4f19f5d1884 100644 --- a/lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php +++ b/lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php @@ -114,12 +114,12 @@ public function orX(...$x): ICompositeExpression { * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string */ - public function comparison($x, string $operator, $y, $type = null): IQueryFunction { + public function comparison($x, string $operator, $y, $type = null): string { $x = $this->helper->quoteColumnName($x); $y = $this->helper->quoteColumnName($y); - return new QueryFunction($this->expressionBuilder->comparison($x, $operator, $y)); + return $this->expressionBuilder->comparison($x, $operator, $y); } /** @@ -137,12 +137,12 @@ public function comparison($x, string $operator, $y, $type = null): IQueryFuncti * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string */ - public function eq($x, $y, $type = null): IQueryFunction { + public function eq($x, $y, $type = null): string { $x = $this->helper->quoteColumnName($x); $y = $this->helper->quoteColumnName($y); - return new QueryFunction($this->expressionBuilder->eq($x, $y)); + return $this->expressionBuilder->eq($x, $y); } /** @@ -159,12 +159,12 @@ public function eq($x, $y, $type = null): IQueryFunction { * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string */ - public function neq($x, $y, $type = null): IQueryFunction { + public function neq($x, $y, $type = null): string { $x = $this->helper->quoteColumnName($x); $y = $this->helper->quoteColumnName($y); - return new QueryFunction($this->expressionBuilder->neq($x, $y)); + return $this->expressionBuilder->neq($x, $y); } /** @@ -181,12 +181,12 @@ public function neq($x, $y, $type = null): IQueryFunction { * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string */ - public function lt($x, $y, $type = null): IQueryFunction { + public function lt($x, $y, $type = null): string { $x = $this->helper->quoteColumnName($x); $y = $this->helper->quoteColumnName($y); - return new QueryFunction($this->expressionBuilder->lt($x, $y)); + return $this->expressionBuilder->lt($x, $y); } /** @@ -203,12 +203,12 @@ public function lt($x, $y, $type = null): IQueryFunction { * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string */ - public function lte($x, $y, $type = null): IQueryFunction { + public function lte($x, $y, $type = null): string { $x = $this->helper->quoteColumnName($x); $y = $this->helper->quoteColumnName($y); - return new QueryFunction($this->expressionBuilder->lte($x, $y)); + return $this->expressionBuilder->lte($x, $y); } /** @@ -225,12 +225,12 @@ public function lte($x, $y, $type = null): IQueryFunction { * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string */ - public function gt($x, $y, $type = null): IQueryFunction { + public function gt($x, $y, $type = null): string { $x = $this->helper->quoteColumnName($x); $y = $this->helper->quoteColumnName($y); - return new QueryFunction($this->expressionBuilder->gt($x, $y)); + return $this->expressionBuilder->gt($x, $y); } /** @@ -247,12 +247,12 @@ public function gt($x, $y, $type = null): IQueryFunction { * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string */ - public function gte($x, $y, $type = null): IQueryFunction { + public function gte($x, $y, $type = null): string { $x = $this->helper->quoteColumnName($x); $y = $this->helper->quoteColumnName($y); - return new QueryFunction($this->expressionBuilder->gte($x, $y)); + return $this->expressionBuilder->gte($x, $y); } /** @@ -260,11 +260,11 @@ public function gte($x, $y, $type = null): IQueryFunction { * * @param string|ILiteral|IParameter|IQueryFunction $x The field in string format to be restricted by IS NULL. * - * @return IQueryFunction + * @return string */ - public function isNull($x): IQueryFunction { + public function isNull($x): string { $x = $this->helper->quoteColumnName($x); - return new QueryFunction($this->expressionBuilder->isNull($x)); + return $this->expressionBuilder->isNull($x); } /** @@ -272,11 +272,11 @@ public function isNull($x): IQueryFunction { * * @param string|ILiteral|IParameter|IQueryFunction $x The field in string format to be restricted by IS NOT NULL. * - * @return IQueryFunction + * @return string */ - public function isNotNull($x): IQueryFunction { + public function isNotNull($x): string { $x = $this->helper->quoteColumnName($x); - return new QueryFunction($this->expressionBuilder->isNotNull($x)); + return $this->expressionBuilder->isNotNull($x); } /** @@ -287,12 +287,12 @@ public function isNotNull($x): IQueryFunction { * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string */ - public function like($x, $y, $type = null): IQueryFunction { + public function like($x, $y, $type = null): string { $x = $this->helper->quoteColumnName($x); $y = $this->helper->quoteColumnName($y); - return new QueryFunction($this->expressionBuilder->like($x, $y)); + return $this->expressionBuilder->like($x, $y); } /** @@ -303,11 +303,11 @@ public function like($x, $y, $type = null): IQueryFunction { * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string * @since 9.0.0 */ - public function iLike($x, $y, $type = null): IQueryFunction { - return new QueryFunction($this->expressionBuilder->like($this->functionBuilder->lower($x), $this->functionBuilder->lower($y))); + public function iLike($x, $y, $type = null): string { + return $this->expressionBuilder->like($this->functionBuilder->lower($x), $this->functionBuilder->lower($y)); } /** @@ -318,12 +318,12 @@ public function iLike($x, $y, $type = null): IQueryFunction { * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string */ - public function notLike($x, $y, $type = null): IQueryFunction { + public function notLike($x, $y, $type = null): string { $x = $this->helper->quoteColumnName($x); $y = $this->helper->quoteColumnName($y); - return new QueryFunction($this->expressionBuilder->notLike($x, $y)); + return $this->expressionBuilder->notLike($x, $y); } /** @@ -334,12 +334,12 @@ public function notLike($x, $y, $type = null): IQueryFunction { * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string */ - public function in($x, $y, $type = null): IQueryFunction { + public function in($x, $y, $type = null): string { $x = $this->helper->quoteColumnName($x); $y = $this->helper->quoteColumnNames($y); - return new QueryFunction($this->expressionBuilder->in($x, $y)); + return $this->expressionBuilder->in($x, $y); } /** @@ -350,34 +350,34 @@ public function in($x, $y, $type = null): IQueryFunction { * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string */ - public function notIn($x, $y, $type = null): IQueryFunction { + public function notIn($x, $y, $type = null): string { $x = $this->helper->quoteColumnName($x); $y = $this->helper->quoteColumnNames($y); - return new QueryFunction($this->expressionBuilder->notIn($x, $y)); + return $this->expressionBuilder->notIn($x, $y); } /** * Creates a $x = '' statement, because Oracle needs a different check * * @param string|ILiteral|IParameter|IQueryFunction $x The field in string format to be inspected by the comparison. - * @return IQueryFunction + * @return string * @since 13.0.0 */ - public function emptyString($x): IQueryFunction { - return new QueryFunction($this->eq($x, $this->literal('', IQueryBuilder::PARAM_STR))); + public function emptyString($x): string { + return $this->eq($x, $this->literal('', IQueryBuilder::PARAM_STR)); } /** * Creates a `$x <> ''` statement, because Oracle needs a different check * * @param string|ILiteral|IParameter|IQueryFunction $x The field in string format to be inspected by the comparison. - * @return IQueryFunction + * @return string * @since 13.0.0 */ - public function nonEmptyString($x): IQueryFunction { - return new QueryFunction($this->neq($x, $this->literal('', IQueryBuilder::PARAM_STR))); + public function nonEmptyString($x): string { + return $this->neq($x, $this->literal('', IQueryBuilder::PARAM_STR)); } /** diff --git a/lib/private/DB/QueryBuilder/ExpressionBuilder/MySqlExpressionBuilder.php b/lib/private/DB/QueryBuilder/ExpressionBuilder/MySqlExpressionBuilder.php index 74209d0c3da38..3bb54d4b26ecc 100644 --- a/lib/private/DB/QueryBuilder/ExpressionBuilder/MySqlExpressionBuilder.php +++ b/lib/private/DB/QueryBuilder/ExpressionBuilder/MySqlExpressionBuilder.php @@ -49,10 +49,10 @@ public function __construct(ConnectionAdapter $connection, IQueryBuilder $queryB /** * @inheritdoc */ - public function iLike($x, $y, $type = null): IQueryFunction { + public function iLike($x, $y, $type = null): string { $x = $this->helper->quoteColumnName($x); $y = $this->helper->quoteColumnName($y); - return new QueryFunction($this->expressionBuilder->comparison($x, ' COLLATE ' . $this->collation . ' LIKE', $y)); + return $this->expressionBuilder->comparison($x, ' COLLATE ' . $this->collation . ' LIKE', $y); } /** diff --git a/lib/private/DB/QueryBuilder/ExpressionBuilder/OCIExpressionBuilder.php b/lib/private/DB/QueryBuilder/ExpressionBuilder/OCIExpressionBuilder.php index 20d68b30b339c..f9b58d7d8eda4 100644 --- a/lib/private/DB/QueryBuilder/ExpressionBuilder/OCIExpressionBuilder.php +++ b/lib/private/DB/QueryBuilder/ExpressionBuilder/OCIExpressionBuilder.php @@ -49,101 +49,101 @@ protected function prepareColumn($column, $type) { /** * @inheritdoc */ - public function comparison($x, string $operator, $y, $type = null): IQueryFunction { + public function comparison($x, string $operator, $y, $type = null): string { $x = $this->prepareColumn($x, $type); $y = $this->prepareColumn($y, $type); - return new QueryFunction($this->expressionBuilder->comparison($x, $operator, $y)); + return $this->expressionBuilder->comparison($x, $operator, $y); } /** * @inheritdoc */ - public function eq($x, $y, $type = null): IQueryFunction { + public function eq($x, $y, $type = null): string { $x = $this->prepareColumn($x, $type); $y = $this->prepareColumn($y, $type); - return new QueryFunction($this->expressionBuilder->eq($x, $y)); + return $this->expressionBuilder->eq($x, $y); } /** * @inheritdoc */ - public function neq($x, $y, $type = null): IQueryFunction { + public function neq($x, $y, $type = null): string { $x = $this->prepareColumn($x, $type); $y = $this->prepareColumn($y, $type); - return new QueryFunction($this->expressionBuilder->neq($x, $y)); + return $this->expressionBuilder->neq($x, $y); } /** * @inheritdoc */ - public function lt($x, $y, $type = null): IQueryFunction { + public function lt($x, $y, $type = null): string { $x = $this->prepareColumn($x, $type); $y = $this->prepareColumn($y, $type); - return new QueryFunction($this->expressionBuilder->lt($x, $y)); + return $this->expressionBuilder->lt($x, $y); } /** * @inheritdoc */ - public function lte($x, $y, $type = null): IQueryFunction { + public function lte($x, $y, $type = null): string { $x = $this->prepareColumn($x, $type); $y = $this->prepareColumn($y, $type); - return new QueryFunction($this->expressionBuilder->lte($x, $y)); + return $this->expressionBuilder->lte($x, $y); } /** * @inheritdoc */ - public function gt($x, $y, $type = null): IQueryFunction { + public function gt($x, $y, $type = null): string { $x = $this->prepareColumn($x, $type); $y = $this->prepareColumn($y, $type); - return new QueryFunction($this->expressionBuilder->gt($x, $y)); + return $this->expressionBuilder->gt($x, $y); } /** * @inheritdoc */ - public function gte($x, $y, $type = null): IQueryFunction { + public function gte($x, $y, $type = null): string { $x = $this->prepareColumn($x, $type); $y = $this->prepareColumn($y, $type); - return new QueryFunction($this->expressionBuilder->gte($x, $y)); + return $this->expressionBuilder->gte($x, $y); } /** * @inheritdoc */ - public function in($x, $y, $type = null): IQueryFunction { + public function in($x, $y, $type = null): string { $x = $this->prepareColumn($x, $type); $y = $this->prepareColumn($y, $type); - return new QueryFunction($this->expressionBuilder->in($x, $y)); + return $this->expressionBuilder->in($x, $y); } /** * @inheritdoc */ - public function notIn($x, $y, $type = null): IQueryFunction { + public function notIn($x, $y, $type = null): string { $x = $this->prepareColumn($x, $type); $y = $this->prepareColumn($y, $type); - return new QueryFunction($this->expressionBuilder->notIn($x, $y)); + return $this->expressionBuilder->notIn($x, $y); } /** * Creates a $x = '' statement, because Oracle needs a different check * * @param string|ILiteral|IParameter|IQueryFunction $x The field in string format to be inspected by the comparison. - * @return IQueryFunction + * @return string * @since 13.0.0 */ - public function emptyString($x): IQueryFunction { + public function emptyString($x): string { return $this->isNull($x); } @@ -151,10 +151,10 @@ public function emptyString($x): IQueryFunction { * Creates a `$x <> ''` statement, because Oracle needs a different check * * @param string|ILiteral|IParameter|IQueryFunction $x The field in string format to be inspected by the comparison. - * @return IQueryFunction + * @return string * @since 13.0.0 */ - public function nonEmptyString($x): IQueryFunction { + public function nonEmptyString($x): string { return $this->isNotNull($x); } @@ -182,14 +182,14 @@ public function castColumn($column, $type): IQueryFunction { /** * @inheritdoc */ - public function like($x, $y, $type = null): IQueryFunction { - return new QueryFunction(parent::like($x, $y, $type) . " ESCAPE '\\'"); + public function like($x, $y, $type = null): string { + return parent::like($x, $y, $type) . " ESCAPE '\\'"; } /** * @inheritdoc */ - public function iLike($x, $y, $type = null): IQueryFunction { + public function iLike($x, $y, $type = null): string { return $this->like($this->functionBuilder->lower($x), $this->functionBuilder->lower($y)); } } diff --git a/lib/private/DB/QueryBuilder/ExpressionBuilder/PgSqlExpressionBuilder.php b/lib/private/DB/QueryBuilder/ExpressionBuilder/PgSqlExpressionBuilder.php index cbebe97ae870a..0fba5363a2831 100644 --- a/lib/private/DB/QueryBuilder/ExpressionBuilder/PgSqlExpressionBuilder.php +++ b/lib/private/DB/QueryBuilder/ExpressionBuilder/PgSqlExpressionBuilder.php @@ -52,9 +52,9 @@ public function castColumn($column, $type): IQueryFunction { /** * @inheritdoc */ - public function iLike($x, $y, $type = null): IQueryFunction { + public function iLike($x, $y, $type = null): string { $x = $this->helper->quoteColumnName($x); $y = $this->helper->quoteColumnName($y); - return new QueryFunction($this->expressionBuilder->comparison($x, 'ILIKE', $y)); + return $this->expressionBuilder->comparison($x, 'ILIKE', $y); } } diff --git a/lib/private/DB/QueryBuilder/ExpressionBuilder/SqliteExpressionBuilder.php b/lib/private/DB/QueryBuilder/ExpressionBuilder/SqliteExpressionBuilder.php index 5425138fa6ca0..289aa09b0039f 100644 --- a/lib/private/DB/QueryBuilder/ExpressionBuilder/SqliteExpressionBuilder.php +++ b/lib/private/DB/QueryBuilder/ExpressionBuilder/SqliteExpressionBuilder.php @@ -23,18 +23,15 @@ */ namespace OC\DB\QueryBuilder\ExpressionBuilder; -use OC\DB\QueryBuilder\QueryFunction; -use OCP\DB\QueryBuilder\IQueryFunction; - class SqliteExpressionBuilder extends ExpressionBuilder { /** * @inheritdoc */ - public function like($x, $y, $type = null): IQueryFunction { - return new QueryFunction(parent::like($x, $y, $type) . " ESCAPE '\\'"); + public function like($x, $y, $type = null): string { + return parent::like($x, $y, $type) . " ESCAPE '\\'"; } - public function iLike($x, $y, $type = null): IQueryFunction { - return new QueryFunction($this->like($this->functionBuilder->lower($x), $this->functionBuilder->lower($y), $type)); + public function iLike($x, $y, $type = null): string { + return $this->like($this->functionBuilder->lower($x), $this->functionBuilder->lower($y), $type); } } diff --git a/lib/private/DB/QueryBuilder/QueryBuilder.php b/lib/private/DB/QueryBuilder/QueryBuilder.php index d991cbd1dd55b..e81ba61b3a76a 100644 --- a/lib/private/DB/QueryBuilder/QueryBuilder.php +++ b/lib/private/DB/QueryBuilder/QueryBuilder.php @@ -715,12 +715,11 @@ public function from($from, $alias = null) { * @param string $fromAlias The alias that points to a from clause. * @param string $join The table name to join. * @param string $alias The alias of the join table. - * @param string|IQueryFunction|ICompositeExpression|null $condition The condition for the join. + * @param string|ICompositeExpression|null $condition The condition for the join. * * @return $this This QueryBuilder instance. */ public function join($fromAlias, $join, $alias, $condition = null) { - $condition = $condition !== null ? (string)$condition : null; $this->queryBuilder->join( $this->quoteAlias($fromAlias), $this->getTableName($join), @@ -744,12 +743,11 @@ public function join($fromAlias, $join, $alias, $condition = null) { * @param string $fromAlias The alias that points to a from clause. * @param string $join The table name to join. * @param string $alias The alias of the join table. - * @param string|IQueryFunction|ICompositeExpression|null $condition The condition for the join. + * @param string|ICompositeExpression|null $condition The condition for the join. * * @return $this This QueryBuilder instance. */ public function innerJoin($fromAlias, $join, $alias, $condition = null) { - $condition = $condition !== null ? (string)$condition : null; $this->queryBuilder->innerJoin( $this->quoteAlias($fromAlias), $this->getTableName($join), @@ -773,12 +771,11 @@ public function innerJoin($fromAlias, $join, $alias, $condition = null) { * @param string $fromAlias The alias that points to a from clause. * @param string $join The table name to join. * @param string $alias The alias of the join table. - * @param string|IQueryFunction|ICompositeExpression|null $condition The condition for the join. + * @param string|ICompositeExpression|null $condition The condition for the join. * * @return $this This QueryBuilder instance. */ public function leftJoin($fromAlias, $join, $alias, $condition = null) { - $condition = $condition !== null ? (string)$condition : null; $this->queryBuilder->leftJoin( $this->quoteAlias($fromAlias), $this->getTableName($join), @@ -802,12 +799,11 @@ public function leftJoin($fromAlias, $join, $alias, $condition = null) { * @param string $fromAlias The alias that points to a from clause. * @param string $join The table name to join. * @param string $alias The alias of the join table. - * @param string|IQueryFunction|ICompositeExpression|null $condition The condition for the join. + * @param string|ICompositeExpression|null $condition The condition for the join. * * @return $this This QueryBuilder instance. */ public function rightJoin($fromAlias, $join, $alias, $condition = null) { - $condition = $condition !== null ? (string)$condition : null; $this->queryBuilder->rightJoin( $this->quoteAlias($fromAlias), $this->getTableName($join), diff --git a/lib/public/DB/QueryBuilder/IExpressionBuilder.php b/lib/public/DB/QueryBuilder/IExpressionBuilder.php index 8f7cd10dee5d0..53dc1fa5a7fa1 100644 --- a/lib/public/DB/QueryBuilder/IExpressionBuilder.php +++ b/lib/public/DB/QueryBuilder/IExpressionBuilder.php @@ -107,7 +107,7 @@ public function orX(...$x): ICompositeExpression; * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string * @since 8.2.0 - Parameter $type was added in 9.0.0 * * @psalm-taint-sink sql $x @@ -115,7 +115,7 @@ public function orX(...$x): ICompositeExpression; * @psalm-taint-sink sql $y * @psalm-taint-sink sql $type */ - public function comparison($x, string $operator, $y, $type = null): IQueryFunction; + public function comparison($x, string $operator, $y, $type = null): string; /** * Creates an equality comparison expression with the given arguments. @@ -132,14 +132,14 @@ public function comparison($x, string $operator, $y, $type = null): IQueryFuncti * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string * @since 8.2.0 - Parameter $type was added in 9.0.0 * * @psalm-taint-sink sql $x * @psalm-taint-sink sql $y * @psalm-taint-sink sql $type */ - public function eq($x, $y, $type = null): IQueryFunction; + public function eq($x, $y, $type = null): string; /** * Creates a non equality comparison expression with the given arguments. @@ -155,14 +155,14 @@ public function eq($x, $y, $type = null): IQueryFunction; * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string * @since 8.2.0 - Parameter $type was added in 9.0.0 * * @psalm-taint-sink sql $x * @psalm-taint-sink sql $y * @psalm-taint-sink sql $type */ - public function neq($x, $y, $type = null): IQueryFunction; + public function neq($x, $y, $type = null): string; /** * Creates a lower-than comparison expression with the given arguments. @@ -178,14 +178,14 @@ public function neq($x, $y, $type = null): IQueryFunction; * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string * @since 8.2.0 - Parameter $type was added in 9.0.0 * * @psalm-taint-sink sql $x * @psalm-taint-sink sql $y * @psalm-taint-sink sql $type */ - public function lt($x, $y, $type = null): IQueryFunction; + public function lt($x, $y, $type = null): string; /** * Creates a lower-than-equal comparison expression with the given arguments. @@ -201,14 +201,14 @@ public function lt($x, $y, $type = null): IQueryFunction; * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string * @since 8.2.0 - Parameter $type was added in 9.0.0 * * @psalm-taint-sink sql $x * @psalm-taint-sink sql $y * @psalm-taint-sink sql $type */ - public function lte($x, $y, $type = null): IQueryFunction; + public function lte($x, $y, $type = null): string; /** * Creates a greater-than comparison expression with the given arguments. @@ -224,14 +224,14 @@ public function lte($x, $y, $type = null): IQueryFunction; * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string * @since 8.2.0 - Parameter $type was added in 9.0.0 * * @psalm-taint-sink sql $x * @psalm-taint-sink sql $y * @psalm-taint-sink sql $type */ - public function gt($x, $y, $type = null): IQueryFunction; + public function gt($x, $y, $type = null): string; /** * Creates a greater-than-equal comparison expression with the given arguments. @@ -247,38 +247,38 @@ public function gt($x, $y, $type = null): IQueryFunction; * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string * @since 8.2.0 - Parameter $type was added in 9.0.0 * * @psalm-taint-sink sql $x * @psalm-taint-sink sql $y * @psalm-taint-sink sql $type */ - public function gte($x, $y, $type = null): IQueryFunction; + public function gte($x, $y, $type = null): string; /** * Creates an IS NULL expression with the given arguments. * * @param string|ILiteral|IParameter|IQueryFunction $x The field in string format to be restricted by IS NULL. * - * @return IQueryFunction + * @return string * @since 8.2.0 * * @psalm-taint-sink sql $x */ - public function isNull($x): IQueryFunction; + public function isNull($x): string; /** * Creates an IS NOT NULL expression with the given arguments. * * @param string|ILiteral|IParameter|IQueryFunction $x The field in string format to be restricted by IS NOT NULL. * - * @return IQueryFunction + * @return string * @since 8.2.0 * * @psalm-taint-sink sql $x */ - public function isNotNull($x): IQueryFunction; + public function isNotNull($x): string; /** * Creates a LIKE() comparison expression with the given arguments. @@ -288,14 +288,14 @@ public function isNotNull($x): IQueryFunction; * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string * @since 8.2.0 - Parameter $type was added in 9.0.0 * * @psalm-taint-sink sql $x * @psalm-taint-sink sql $y * @psalm-taint-sink sql $type */ - public function like($x, $y, $type = null): IQueryFunction; + public function like($x, $y, $type = null): string; /** * Creates a NOT LIKE() comparison expression with the given arguments. @@ -305,14 +305,14 @@ public function like($x, $y, $type = null): IQueryFunction; * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string * @since 8.2.0 - Parameter $type was added in 9.0.0 * * @psalm-taint-sink sql $x * @psalm-taint-sink sql $y * @psalm-taint-sink sql $type */ - public function notLike($x, $y, $type = null): IQueryFunction; + public function notLike($x, $y, $type = null): string; /** * Creates a ILIKE() comparison expression with the given arguments. @@ -322,14 +322,14 @@ public function notLike($x, $y, $type = null): IQueryFunction; * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string * @since 9.0.0 * * @psalm-taint-sink sql $x * @psalm-taint-sink sql $y * @psalm-taint-sink sql $type */ - public function iLike($x, $y, $type = null): IQueryFunction; + public function iLike($x, $y, $type = null): string; /** * Creates a IN () comparison expression with the given arguments. @@ -339,14 +339,14 @@ public function iLike($x, $y, $type = null): IQueryFunction; * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string * @since 8.2.0 - Parameter $type was added in 9.0.0 * * @psalm-taint-sink sql $x * @psalm-taint-sink sql $y * @psalm-taint-sink sql $type */ - public function in($x, $y, $type = null): IQueryFunction; + public function in($x, $y, $type = null): string; /** * Creates a NOT IN () comparison expression with the given arguments. @@ -356,36 +356,36 @@ public function in($x, $y, $type = null): IQueryFunction; * @param mixed|null $type one of the IQueryBuilder::PARAM_* constants * required when comparing text fields for oci compatibility * - * @return IQueryFunction + * @return string * @since 8.2.0 - Parameter $type was added in 9.0.0 * * @psalm-taint-sink sql $x * @psalm-taint-sink sql $y * @psalm-taint-sink sql $type */ - public function notIn($x, $y, $type = null): IQueryFunction; + public function notIn($x, $y, $type = null): string; /** * Creates a $x = '' statement, because Oracle needs a different check * * @param string|ILiteral|IParameter|IQueryFunction $x The field in string format to be inspected by the comparison. - * @return IQueryFunction + * @return string * @since 13.0.0 * * @psalm-taint-sink sql $x */ - public function emptyString($x): IQueryFunction; + public function emptyString($x): string; /** * Creates a `$x <> ''` statement, because Oracle needs a different check * * @param string|ILiteral|IParameter|IQueryFunction $x The field in string format to be inspected by the comparison. - * @return IQueryFunction + * @return string * @since 13.0.0 * * @psalm-taint-sink sql $x */ - public function nonEmptyString($x): IQueryFunction; + public function nonEmptyString($x): string; /** diff --git a/lib/public/DB/QueryBuilder/IQueryBuilder.php b/lib/public/DB/QueryBuilder/IQueryBuilder.php index 446e9a00b4c0a..218b7d8cb2d6c 100644 --- a/lib/public/DB/QueryBuilder/IQueryBuilder.php +++ b/lib/public/DB/QueryBuilder/IQueryBuilder.php @@ -503,7 +503,7 @@ public function from($from, $alias = null); * @param string $fromAlias The alias that points to a from clause. * @param string $join The table name to join. * @param string $alias The alias of the join table. - * @param string|IQueryFunction|ICompositeExpression|null $condition The condition for the join. + * @param string|ICompositeExpression|null $condition The condition for the join. * * @return $this This QueryBuilder instance. * @since 8.2.0 @@ -528,7 +528,7 @@ public function join($fromAlias, $join, $alias, $condition = null); * @param string $fromAlias The alias that points to a from clause. * @param string $join The table name to join. * @param string $alias The alias of the join table. - * @param string|IQueryFunction|ICompositeExpression|null $condition The condition for the join. + * @param string|ICompositeExpression|null $condition The condition for the join. * * @return $this This QueryBuilder instance. * @since 8.2.0 @@ -553,7 +553,7 @@ public function innerJoin($fromAlias, $join, $alias, $condition = null); * @param string $fromAlias The alias that points to a from clause. * @param string $join The table name to join. * @param string $alias The alias of the join table. - * @param string|IQueryFunction|ICompositeExpression|null $condition The condition for the join. + * @param string|ICompositeExpression|null $condition The condition for the join. * * @return $this This QueryBuilder instance. * @since 8.2.0 @@ -578,7 +578,7 @@ public function leftJoin($fromAlias, $join, $alias, $condition = null); * @param string $fromAlias The alias that points to a from clause. * @param string $join The table name to join. * @param string $alias The alias of the join table. - * @param string|IQueryFunction|ICompositeExpression|null $condition The condition for the join. + * @param string|ICompositeExpression|null $condition The condition for the join. * * @return $this This QueryBuilder instance. * @since 8.2.0 From 6e0123a1d0343b6b850ef6e3b341acf082fa0dce Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 20 Jun 2022 17:53:31 +0200 Subject: [PATCH 2/5] Revert "add case statement to sql function builder" This reverts commit 2a68819a67045d87a369a8a6413f153b3b2bea5f. Signed-off-by: Robin Appelman --- .../FunctionBuilder/FunctionBuilder.php | 11 ----------- lib/public/DB/QueryBuilder/IFunctionBuilder.php | 12 ------------ .../lib/DB/QueryBuilder/FunctionBuilderTest.php | 17 ----------------- 3 files changed, 40 deletions(-) diff --git a/lib/private/DB/QueryBuilder/FunctionBuilder/FunctionBuilder.php b/lib/private/DB/QueryBuilder/FunctionBuilder/FunctionBuilder.php index 408a879d624ad..e0a7549a0ad43 100644 --- a/lib/private/DB/QueryBuilder/FunctionBuilder/FunctionBuilder.php +++ b/lib/private/DB/QueryBuilder/FunctionBuilder/FunctionBuilder.php @@ -121,15 +121,4 @@ public function greatest($x, $y): IQueryFunction { public function least($x, $y): IQueryFunction { return new QueryFunction('LEAST(' . $this->helper->quoteColumnName($x) . ', ' . $this->helper->quoteColumnName($y) . ')'); } - - public function case(array $whens, $else): IQueryFunction { - if (count($whens) < 1) { - return new QueryFunction($this->helper->quoteColumnName($else)); - } - - $whenParts = array_map(function (array $when) { - return 'WHEN ' . $this->helper->quoteColumnName($when['when']) . ' THEN ' . $this->helper->quoteColumnName($when['then']); - }, $whens); - return new QueryFunction('CASE ' . implode(' ', $whenParts) . ' ELSE ' . $this->helper->quoteColumnName($else) . ' END'); - } } diff --git a/lib/public/DB/QueryBuilder/IFunctionBuilder.php b/lib/public/DB/QueryBuilder/IFunctionBuilder.php index 811e8d06aaf90..d4edc8ea9f814 100644 --- a/lib/public/DB/QueryBuilder/IFunctionBuilder.php +++ b/lib/public/DB/QueryBuilder/IFunctionBuilder.php @@ -188,16 +188,4 @@ public function greatest($x, $y): IQueryFunction; * @since 18.0.0 */ public function least($x, $y): IQueryFunction; - - /** - * Takes the minimum of multiple values - * - * If you want to get the minimum value of all rows in a column, use `min` instead - * - * @param array $whens - * @param string|ILiteral|IParameter|IQueryFunction $else - * @return IQueryFunction - * @since 18.0.0 - */ - public function case(array $whens, $else): IQueryFunction; } diff --git a/tests/lib/DB/QueryBuilder/FunctionBuilderTest.php b/tests/lib/DB/QueryBuilder/FunctionBuilderTest.php index 0ea6e69c95639..08392b09d8dd2 100644 --- a/tests/lib/DB/QueryBuilder/FunctionBuilderTest.php +++ b/tests/lib/DB/QueryBuilder/FunctionBuilderTest.php @@ -501,21 +501,4 @@ public function testLeast() { $result->closeCursor(); $this->assertEquals(1, $row); } - - public function testCase() { - $query = $this->connection->getQueryBuilder(); - - $query->select($query->func()->case([ - ['when' => $query->expr()->gt($query->expr()->literal(1, IQueryBuilder::PARAM_INT), $query->expr()->literal(2, IQueryBuilder::PARAM_INT)), 'then' => $query->expr()->literal('first')], - ['when' => $query->expr()->lt($query->expr()->literal(1, IQueryBuilder::PARAM_INT), $query->expr()->literal(2, IQueryBuilder::PARAM_INT)), 'then' => $query->expr()->literal('second')], - ['when' => $query->expr()->eq($query->expr()->literal(1, IQueryBuilder::PARAM_INT), $query->expr()->literal(2, IQueryBuilder::PARAM_INT)), 'then' => $query->expr()->literal('third')], - ], $query->createNamedParameter('else'))); - $query->from('appconfig') - ->setMaxResults(1); - - $result = $query->execute(); - $row = $result->fetchOne(); - $result->closeCursor(); - $this->assertEquals('second', $row); - } } From 5e375d9092efd1e40a0ed37dfd2c208598b27bb9 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 20 Jun 2022 17:56:59 +0200 Subject: [PATCH 3/5] Revert "store unencrypted size in the unencrypted_size column" This reverts commit 8238582e59b7b6ec03318bcf81bf47cce54af320. Signed-off-by: Robin Appelman --- lib/private/Files/Cache/Cache.php | 35 +------ lib/private/Files/Cache/CacheEntry.php | 8 -- lib/private/Files/Cache/Propagator.php | 15 --- lib/private/Files/FileInfo.php | 21 +--- .../Files/Storage/Wrapper/Encryption.php | 98 ++++++++----------- lib/public/Files/Cache/ICacheEntry.php | 10 -- .../Files/Storage/Wrapper/EncryptionTest.php | 7 +- tests/lib/HelperStorageTest.php | 3 - 8 files changed, 50 insertions(+), 147 deletions(-) diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index f23635aa01bd5..9a7ffdab025f6 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -37,7 +37,6 @@ * along with this program. If not, see * */ - namespace OC\Files\Cache; use Doctrine\DBAL\Exception\UniqueConstraintViolationException; @@ -189,7 +188,6 @@ public static function cacheEntryFromData($data, IMimeTypeLoader $mimetypeLoader $data['fileid'] = (int)$data['fileid']; $data['parent'] = (int)$data['parent']; $data['size'] = 0 + $data['size']; - $data['unencrypted_size'] = 0 + ($data['unencrypted_size'] ?? 0); $data['mtime'] = (int)$data['mtime']; $data['storage_mtime'] = (int)$data['storage_mtime']; $data['encryptedVersion'] = (int)$data['encrypted']; @@ -430,7 +428,7 @@ public function update($id, array $data) { protected function normalizeData(array $data): array { $fields = [ 'path', 'parent', 'name', 'mimetype', 'size', 'mtime', 'storage_mtime', 'encrypted', - 'etag', 'permissions', 'checksum', 'storage', 'unencrypted_size']; + 'etag', 'permissions', 'checksum', 'storage']; $extensionFields = ['metadata_etag', 'creation_time', 'upload_time']; $doNotCopyStorageMTime = false; @@ -875,16 +873,8 @@ public function calculateFolderSize($path, $entry = null) { $id = $entry['fileid']; $query = $this->getQueryBuilder(); - $query->selectAlias($query->func()->sum('size'), 'size_sum') - ->selectAlias($query->func()->min('size'), 'size_min') - // in case of encryption being enabled after some files are already uploaded, some entries will have an unencrypted_size of 0 and a non-zero size - ->selectAlias($query->func()->sum( - $query->func()->case([ - ['when' => $query->expr()->eq('unencrypted_size', $query->expr()->literal(0, IQueryBuilder::PARAM_INT)), 'then' => 'size'], - ], 'unencrypted_size') - ), 'unencrypted_sum') - ->selectAlias($query->func()->min('unencrypted_size'), 'unencrypted_min') - ->selectAlias($query->func()->max('unencrypted_size'), 'unencrypted_max') + $query->selectAlias($query->func()->sum('size'), 'f1') + ->selectAlias($query->func()->min('size'), 'f2') ->from('filecache') ->whereStorageId($this->getNumericStorageId()) ->whereParent($id); @@ -894,7 +884,7 @@ public function calculateFolderSize($path, $entry = null) { $result->closeCursor(); if ($row) { - ['size_sum' => $sum, 'size_min' => $min, 'unencrypted_sum' => $unencryptedSum, 'unencrypted_min' => $unencryptedMin, 'unencrypted_max' => $unencryptedMax] = $row; + [$sum, $min] = array_values($row); $sum = 0 + $sum; $min = 0 + $min; if ($min === -1) { @@ -902,23 +892,8 @@ public function calculateFolderSize($path, $entry = null) { } else { $totalSize = $sum; } - if ($unencryptedMin === -1 || $min === -1) { - $unencryptedTotal = $unencryptedMin; - } else { - $unencryptedTotal = $unencryptedSum; - } if ($entry['size'] !== $totalSize) { - // only set unencrypted size for a folder if any child entries have it set - if ($unencryptedMax > 0) { - $this->update($id, [ - 'size' => $totalSize, - 'unencrypted_size' => $unencryptedTotal, - ]); - } else { - $this->update($id, [ - 'size' => $totalSize, - ]); - } + $this->update($id, ['size' => $totalSize]); } } } diff --git a/lib/private/Files/Cache/CacheEntry.php b/lib/private/Files/Cache/CacheEntry.php index 8ac76acf6d140..12f0273fb6e7e 100644 --- a/lib/private/Files/Cache/CacheEntry.php +++ b/lib/private/Files/Cache/CacheEntry.php @@ -132,12 +132,4 @@ public function getData() { public function __clone() { $this->data = array_merge([], $this->data); } - - public function getUnencryptedSize(): int { - if (isset($this->data['unencrypted_size']) && $this->data['unencrypted_size'] > 0) { - return $this->data['unencrypted_size']; - } else { - return $this->data['size']; - } - } } diff --git a/lib/private/Files/Cache/Propagator.php b/lib/private/Files/Cache/Propagator.php index a0953baa78587..63117154d19e6 100644 --- a/lib/private/Files/Cache/Propagator.php +++ b/lib/private/Files/Cache/Propagator.php @@ -24,7 +24,6 @@ namespace OC\Files\Cache; -use OC\Files\Storage\Wrapper\Encryption; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\IPropagator; use OCP\Files\Storage\IReliableEtagStorage; @@ -114,20 +113,6 @@ public function propagateChange($internalPath, $time, $sizeDifference = 0) { ->andWhere($builder->expr()->in('path_hash', $hashParams)) ->andWhere($builder->expr()->gt('size', $builder->expr()->literal(-1, IQueryBuilder::PARAM_INT))); - if ($this->storage->instanceOfStorage(Encryption::class)) { - // in case of encryption being enabled after some files are already uploaded, some entries will have an unencrypted_size of 0 and a non-zero size - $builder->set('unencrypted_size', $builder->func()->greatest( - $builder->func()->add( - $builder->func()->case([ - ['when' => $builder->expr()->eq('unencrypted_size', $builder->expr()->literal(0, IQueryBuilder::PARAM_INT)), 'then' => 'size'] - ], 'unencrypted_size'), - $builder->createNamedParameter($sizeDifference) - ), - $builder->createNamedParameter(-1, IQueryBuilder::PARAM_INT) - )); - } - - $a = $builder->getSQL(); $builder->execute(); } } diff --git a/lib/private/Files/FileInfo.php b/lib/private/Files/FileInfo.php index 47c893ebbf153..21197e016ca48 100644 --- a/lib/private/Files/FileInfo.php +++ b/lib/private/Files/FileInfo.php @@ -212,12 +212,7 @@ public function getEtag() { public function getSize($includeMounts = true) { if ($includeMounts) { $this->updateEntryfromSubMounts(); - - if (isset($this->data['unencrypted_size']) && $this->data['unencrypted_size'] > 0) { - return $this->data['unencrypted_size']; - } else { - return isset($this->data['size']) ? 0 + $this->data['size'] : 0; - } + return isset($this->data['size']) ? 0 + $this->data['size'] : 0; } else { return $this->rawSize; } @@ -395,19 +390,7 @@ private function updateEntryfromSubMounts() { * @param string $entryPath full path of the child entry */ public function addSubEntry($data, $entryPath) { - if (!$data) { - return; - } - $hasUnencryptedSize = isset($data['unencrypted_size']) && $data['unencrypted_size'] > 0; - if ($hasUnencryptedSize) { - $subSize = $data['unencrypted_size']; - } else { - $subSize = $data['size'] ?: 0; - } - $this->data['size'] += $subSize; - if ($hasUnencryptedSize) { - $this->data['unencrypted_size'] += $subSize; - } + $this->data['size'] += isset($data['size']) ? $data['size'] : 0; if (isset($data['mtime'])) { $this->data['mtime'] = max($this->data['mtime'], $data['mtime']); } diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index d5bf929101fd9..4cfe932cc9ff7 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -33,7 +33,6 @@ * along with this program. If not, see * */ - namespace OC\Files\Storage\Wrapper; use OC\Encryption\Exceptions\ModuleDoesNotExistsException; @@ -42,7 +41,6 @@ use OC\Files\Cache\CacheEntry; use OC\Files\Filesystem; use OC\Files\Mount\Manager; -use OC\Files\ObjectStore\ObjectStoreStorage; use OC\Files\Storage\LocalTempFileTrait; use OC\Memcache\ArrayCache; use OCP\Encryption\Exceptions\GenericEncryptionException; @@ -141,36 +139,28 @@ public function filesize($path) { $size = $this->unencryptedSize[$fullPath]; // update file cache if ($info instanceof ICacheEntry) { + $info = $info->getData(); $info['encrypted'] = $info['encryptedVersion']; } else { if (!is_array($info)) { $info = []; } $info['encrypted'] = true; - $info = new CacheEntry($info); } - if ($size !== $info->getUnencryptedSize()) { - $this->getCache()->update($info->getId(), [ - 'unencrypted_size' => $size - ]); - } + $info['size'] = $size; + $this->getCache()->put($path, $info); return $size; } if (isset($info['fileid']) && $info['encrypted']) { - return $this->verifyUnencryptedSize($path, $info->getUnencryptedSize()); + return $this->verifyUnencryptedSize($path, $info['size']); } return $this->storage->filesize($path); } - /** - * @param string $path - * @param array $data - * @return array - */ private function modifyMetaData(string $path, array $data): array { $fullPath = $this->getFullPath($path); $info = $this->getCache()->get($path); @@ -180,7 +170,7 @@ private function modifyMetaData(string $path, array $data): array { $data['size'] = $this->unencryptedSize[$fullPath]; } else { if (isset($info['fileid']) && $info['encrypted']) { - $data['size'] = $this->verifyUnencryptedSize($path, $info->getUnencryptedSize()); + $data['size'] = $this->verifyUnencryptedSize($path, $info['size']); $data['encrypted'] = true; } } @@ -488,7 +478,7 @@ public function fopen($path, $mode) { * * @return int unencrypted size */ - protected function verifyUnencryptedSize(string $path, int $unencryptedSize): int { + protected function verifyUnencryptedSize($path, $unencryptedSize) { $size = $this->storage->filesize($path); $result = $unencryptedSize; @@ -520,7 +510,7 @@ protected function verifyUnencryptedSize(string $path, int $unencryptedSize): in * * @return int calculated unencrypted size */ - protected function fixUnencryptedSize(string $path, int $size, int $unencryptedSize): int { + protected function fixUnencryptedSize($path, $size, $unencryptedSize) { $headerSize = $this->getHeaderSize($path); $header = $this->getHeader($path); $encryptionModule = $this->getEncryptionModule($path); @@ -591,9 +581,7 @@ protected function fixUnencryptedSize(string $path, int $size, int $unencryptedS $cache = $this->storage->getCache(); if ($cache) { $entry = $cache->get($path); - $cache->update($entry['fileid'], [ - 'unencrypted_size' => $newUnencryptedSize - ]); + $cache->update($entry['fileid'], ['size' => $newUnencryptedSize]); } return $newUnencryptedSize; @@ -633,12 +621,7 @@ private function fread_block($handle, int $blockSize): string { * @param bool $preserveMtime * @return bool */ - public function moveFromStorage( - Storage\IStorage $sourceStorage, - $sourceInternalPath, - $targetInternalPath, - $preserveMtime = true - ) { + public function moveFromStorage(Storage\IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime = true) { if ($sourceStorage === $this) { return $this->rename($sourceInternalPath, $targetInternalPath); } @@ -673,13 +656,7 @@ public function moveFromStorage( * @param bool $isRename * @return bool */ - public function copyFromStorage( - Storage\IStorage $sourceStorage, - $sourceInternalPath, - $targetInternalPath, - $preserveMtime = false, - $isRename = false - ) { + public function copyFromStorage(Storage\IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime = false, $isRename = false) { // TODO clean this up once the underlying moveFromStorage in OC\Files\Storage\Wrapper\Common is fixed: // - call $this->storage->copyFromStorage() instead of $this->copyBetweenStorage @@ -699,13 +676,7 @@ public function copyFromStorage( * @param bool $isRename * @param bool $keepEncryptionVersion */ - private function updateEncryptedVersion( - Storage\IStorage $sourceStorage, - $sourceInternalPath, - $targetInternalPath, - $isRename, - $keepEncryptionVersion - ) { + private function updateEncryptedVersion(Storage\IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath, $isRename, $keepEncryptionVersion) { $isEncrypted = $this->encryptionManager->isEnabled() && $this->shouldEncrypt($targetInternalPath); $cacheInformation = [ 'encrypted' => $isEncrypted, @@ -754,13 +725,7 @@ private function updateEncryptedVersion( * @return bool * @throws \Exception */ - private function copyBetweenStorage( - Storage\IStorage $sourceStorage, - $sourceInternalPath, - $targetInternalPath, - $preserveMtime, - $isRename - ) { + private function copyBetweenStorage(Storage\IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime, $isRename) { // for versions we have nothing to do, because versions should always use the // key from the original file. Just create a 1:1 copy and done @@ -778,7 +743,7 @@ private function copyBetweenStorage( if (isset($info['encrypted']) && $info['encrypted'] === true) { $this->updateUnencryptedSize( $this->getFullPath($targetInternalPath), - $info->getUnencryptedSize() + $info['size'] ); } $this->updateEncryptedVersion($sourceStorage, $sourceInternalPath, $targetInternalPath, $isRename, true); @@ -843,6 +808,13 @@ private function copyBetweenStorage( return (bool)$result; } + /** + * get the path to a local version of the file. + * The local version of the file can be temporary and doesn't have to be persistent across requests + * + * @param string $path + * @return string + */ public function getLocalFile($path) { if ($this->encryptionManager->isEnabled()) { $cachedFile = $this->getCachedFile($path); @@ -853,6 +825,11 @@ public function getLocalFile($path) { return $this->storage->getLocalFile($path); } + /** + * Returns the wrapped storage's value for isLocal() + * + * @return bool wrapped storage's isLocal() value + */ public function isLocal() { if ($this->encryptionManager->isEnabled()) { return false; @@ -860,11 +837,15 @@ public function isLocal() { return $this->storage->isLocal(); } + /** + * see https://www.php.net/manual/en/function.stat.php + * only the following keys are required in the result: size and mtime + * + * @param string $path + * @return array + */ public function stat($path) { $stat = $this->storage->stat($path); - if (!$stat) { - return false; - } $fileSize = $this->filesize($path); $stat['size'] = $fileSize; $stat[7] = $fileSize; @@ -872,6 +853,14 @@ public function stat($path) { return $stat; } + /** + * see https://www.php.net/manual/en/function.hash.php + * + * @param string $type + * @param string $path + * @param bool $raw + * @return string + */ public function hash($type, $path, $raw = false) { $fh = $this->fopen($path, 'rb'); $ctx = hash_init($type); @@ -1079,13 +1068,6 @@ public function writeStream(string $path, $stream, int $size = null): int { [$count, $result] = \OC_Helper::streamCopy($stream, $target); fclose($stream); fclose($target); - - // object store, stores the size after write and doesn't update this during scan - // manually store the unencrypted size - if ($result && $this->getWrapperStorage()->instanceOfStorage(ObjectStoreStorage::class)) { - $this->getCache()->put($path, ['unencrypted_size' => $count]); - } - return $count; } } diff --git a/lib/public/Files/Cache/ICacheEntry.php b/lib/public/Files/Cache/ICacheEntry.php index e1e8129394c71..17eecf89ddb2f 100644 --- a/lib/public/Files/Cache/ICacheEntry.php +++ b/lib/public/Files/Cache/ICacheEntry.php @@ -162,14 +162,4 @@ public function getCreationTime(): ?int; * @since 18.0.0 */ public function getUploadTime(): ?int; - - /** - * Get the unencrypted size - * - * This might be different from the result of getSize - * - * @return int - * @since 25.0.0 - */ - public function getUnencryptedSize(): int; } diff --git a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php index ebb97a25c77e0..d26e5c499e70b 100644 --- a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php +++ b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php @@ -5,7 +5,6 @@ use OC\Encryption\Exceptions\ModuleDoesNotExistsException; use OC\Encryption\Update; use OC\Encryption\Util; -use OC\Files\Cache\CacheEntry; use OC\Files\Storage\Temporary; use OC\Files\Storage\Wrapper\Encryption; use OC\Files\View; @@ -260,7 +259,7 @@ public function testGetMetaData($path, $metaData, $encrypted, $unencryptedSizeSe ->method('get') ->willReturnCallback( function ($path) use ($encrypted) { - return new CacheEntry(['encrypted' => $encrypted, 'path' => $path, 'size' => 0, 'fileid' => 1]); + return ['encrypted' => $encrypted, 'path' => $path, 'size' => 0, 'fileid' => 1]; } ); @@ -333,7 +332,7 @@ public function testFilesize() { ->disableOriginalConstructor()->getMock(); $cache->expects($this->any()) ->method('get') - ->willReturn(new CacheEntry(['encrypted' => true, 'path' => '/test.txt', 'size' => 0, 'fileid' => 1])); + ->willReturn(['encrypted' => true, 'path' => '/test.txt', 'size' => 0, 'fileid' => 1]); $this->instance = $this->getMockBuilder('\OC\Files\Storage\Wrapper\Encryption') ->setConstructorArgs( @@ -911,7 +910,7 @@ public function testCopyBetweenStorageVersions($sourceInternalPath, $targetInter if ($copyResult) { $cache->expects($this->once())->method('get') ->with($sourceInternalPath) - ->willReturn(new CacheEntry(['encrypted' => $encrypted, 'size' => 42])); + ->willReturn(['encrypted' => $encrypted, 'size' => 42]); if ($encrypted) { $instance->expects($this->once())->method('updateUnencryptedSize') ->with($mountPoint . $targetInternalPath, 42); diff --git a/tests/lib/HelperStorageTest.php b/tests/lib/HelperStorageTest.php index d3f480502b2d8..6d7ea513d3f49 100644 --- a/tests/lib/HelperStorageTest.php +++ b/tests/lib/HelperStorageTest.php @@ -104,9 +104,6 @@ public function testGetStorageInfoExcludingExtStorage() { $extStorage->file_put_contents('extfile.txt', 'abcdefghijklmnopq'); $extStorage->getScanner()->scan(''); // update root size - $config = \OC::$server->getConfig(); - $config->setSystemValue('quota_include_external_storage', false); - \OC\Files\Filesystem::mount($extStorage, [], '/' . $this->user . '/files/ext'); $storageInfo = \OC_Helper::getStorageInfo(''); From 1374cbee3e5d44ecec0a0784b3ab3a5afcc0d2ac Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 13 Apr 2022 16:05:45 +0200 Subject: [PATCH 4/5] store unencrypted size in the unencrypted_size column Signed-off-by: Robin Appelman --- lib/private/Files/Cache/Cache.php | 47 +++++++-- lib/private/Files/Cache/CacheEntry.php | 8 ++ lib/private/Files/Cache/Propagator.php | 15 +++ lib/private/Files/FileInfo.php | 21 +++- .../Files/Storage/Wrapper/Encryption.php | 98 +++++++++++-------- lib/public/Files/Cache/ICacheEntry.php | 10 ++ tests/lib/Files/ObjectStore/S3Test.php | 2 +- .../Files/Storage/Wrapper/EncryptionTest.php | 7 +- tests/lib/HelperStorageTest.php | 3 + 9 files changed, 157 insertions(+), 54 deletions(-) diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index 9a7ffdab025f6..5100889637332 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -37,6 +37,7 @@ * along with this program. If not, see * */ + namespace OC\Files\Cache; use Doctrine\DBAL\Exception\UniqueConstraintViolationException; @@ -188,6 +189,7 @@ public static function cacheEntryFromData($data, IMimeTypeLoader $mimetypeLoader $data['fileid'] = (int)$data['fileid']; $data['parent'] = (int)$data['parent']; $data['size'] = 0 + $data['size']; + $data['unencrypted_size'] = 0 + ($data['unencrypted_size'] ?? 0); $data['mtime'] = (int)$data['mtime']; $data['storage_mtime'] = (int)$data['storage_mtime']; $data['encryptedVersion'] = (int)$data['encrypted']; @@ -428,7 +430,7 @@ public function update($id, array $data) { protected function normalizeData(array $data): array { $fields = [ 'path', 'parent', 'name', 'mimetype', 'size', 'mtime', 'storage_mtime', 'encrypted', - 'etag', 'permissions', 'checksum', 'storage']; + 'etag', 'permissions', 'checksum', 'storage', 'unencrypted_size']; $extensionFields = ['metadata_etag', 'creation_time', 'upload_time']; $doNotCopyStorageMTime = false; @@ -873,18 +875,32 @@ public function calculateFolderSize($path, $entry = null) { $id = $entry['fileid']; $query = $this->getQueryBuilder(); - $query->selectAlias($query->func()->sum('size'), 'f1') - ->selectAlias($query->func()->min('size'), 'f2') + $query->select('size', 'unencrypted_size') ->from('filecache') - ->whereStorageId($this->getNumericStorageId()) ->whereParent($id); $result = $query->execute(); - $row = $result->fetch(); + $rows = $result->fetchAll(); $result->closeCursor(); - if ($row) { - [$sum, $min] = array_values($row); + if ($rows) { + $sizes = array_map(function (array $row) { + return (int)$row['size']; + }, $rows); + $unencryptedOnlySizes = array_map(function (array $row) { + return (int)$row['unencrypted_size']; + }, $rows); + $unencryptedSizes = array_map(function (array $row) { + return (int)(($row['unencrypted_size'] > 0) ? $row['unencrypted_size']: $row['size']); + }, $rows); + + $sum = array_sum($sizes); + $min = min($sizes); + + $unencryptedSum = array_sum($unencryptedSizes); + $unencryptedMin = min($unencryptedSizes); + $unencryptedMax = max($unencryptedOnlySizes); + $sum = 0 + $sum; $min = 0 + $min; if ($min === -1) { @@ -892,8 +908,23 @@ public function calculateFolderSize($path, $entry = null) { } else { $totalSize = $sum; } + if ($unencryptedMin === -1 || $min === -1) { + $unencryptedTotal = $unencryptedMin; + } else { + $unencryptedTotal = $unencryptedSum; + } if ($entry['size'] !== $totalSize) { - $this->update($id, ['size' => $totalSize]); + // only set unencrypted size for a folder if any child entries have it set + if ($unencryptedMax > 0) { + $this->update($id, [ + 'size' => $totalSize, + 'unencrypted_size' => $unencryptedTotal, + ]); + } else { + $this->update($id, [ + 'size' => $totalSize, + ]); + } } } } diff --git a/lib/private/Files/Cache/CacheEntry.php b/lib/private/Files/Cache/CacheEntry.php index 12f0273fb6e7e..8ac76acf6d140 100644 --- a/lib/private/Files/Cache/CacheEntry.php +++ b/lib/private/Files/Cache/CacheEntry.php @@ -132,4 +132,12 @@ public function getData() { public function __clone() { $this->data = array_merge([], $this->data); } + + public function getUnencryptedSize(): int { + if (isset($this->data['unencrypted_size']) && $this->data['unencrypted_size'] > 0) { + return $this->data['unencrypted_size']; + } else { + return $this->data['size']; + } + } } diff --git a/lib/private/Files/Cache/Propagator.php b/lib/private/Files/Cache/Propagator.php index 63117154d19e6..2909e998bf97f 100644 --- a/lib/private/Files/Cache/Propagator.php +++ b/lib/private/Files/Cache/Propagator.php @@ -24,6 +24,7 @@ namespace OC\Files\Cache; +use OC\Files\Storage\Wrapper\Encryption; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Files\Cache\IPropagator; use OCP\Files\Storage\IReliableEtagStorage; @@ -113,6 +114,20 @@ public function propagateChange($internalPath, $time, $sizeDifference = 0) { ->andWhere($builder->expr()->in('path_hash', $hashParams)) ->andWhere($builder->expr()->gt('size', $builder->expr()->literal(-1, IQueryBuilder::PARAM_INT))); + if ($this->storage->instanceOfStorage(Encryption::class)) { + // in case of encryption being enabled after some files are already uploaded, some entries will have an unencrypted_size of 0 and a non-zero size + $eq = $builder->expr()->eq('unencrypted_size', $builder->expr()->literal(0, IQueryBuilder::PARAM_INT)); + $sizeColumn = $builder->getColumnName('size'); + $unencryptedSizeColumn = $builder->getColumnName('unencrypted_size'); + $builder->set('unencrypted_size', $builder->func()->greatest( + $builder->func()->add( + $builder->createFunction("CASE WHEN $eq THEN $unencryptedSizeColumn ELSE $sizeColumn END"), + $builder->createNamedParameter($sizeDifference) + ), + $builder->createNamedParameter(-1, IQueryBuilder::PARAM_INT) + )); + } + $builder->execute(); } } diff --git a/lib/private/Files/FileInfo.php b/lib/private/Files/FileInfo.php index 21197e016ca48..47c893ebbf153 100644 --- a/lib/private/Files/FileInfo.php +++ b/lib/private/Files/FileInfo.php @@ -212,7 +212,12 @@ public function getEtag() { public function getSize($includeMounts = true) { if ($includeMounts) { $this->updateEntryfromSubMounts(); - return isset($this->data['size']) ? 0 + $this->data['size'] : 0; + + if (isset($this->data['unencrypted_size']) && $this->data['unencrypted_size'] > 0) { + return $this->data['unencrypted_size']; + } else { + return isset($this->data['size']) ? 0 + $this->data['size'] : 0; + } } else { return $this->rawSize; } @@ -390,7 +395,19 @@ private function updateEntryfromSubMounts() { * @param string $entryPath full path of the child entry */ public function addSubEntry($data, $entryPath) { - $this->data['size'] += isset($data['size']) ? $data['size'] : 0; + if (!$data) { + return; + } + $hasUnencryptedSize = isset($data['unencrypted_size']) && $data['unencrypted_size'] > 0; + if ($hasUnencryptedSize) { + $subSize = $data['unencrypted_size']; + } else { + $subSize = $data['size'] ?: 0; + } + $this->data['size'] += $subSize; + if ($hasUnencryptedSize) { + $this->data['unencrypted_size'] += $subSize; + } if (isset($data['mtime'])) { $this->data['mtime'] = max($this->data['mtime'], $data['mtime']); } diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index 4cfe932cc9ff7..d5bf929101fd9 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -33,6 +33,7 @@ * along with this program. If not, see * */ + namespace OC\Files\Storage\Wrapper; use OC\Encryption\Exceptions\ModuleDoesNotExistsException; @@ -41,6 +42,7 @@ use OC\Files\Cache\CacheEntry; use OC\Files\Filesystem; use OC\Files\Mount\Manager; +use OC\Files\ObjectStore\ObjectStoreStorage; use OC\Files\Storage\LocalTempFileTrait; use OC\Memcache\ArrayCache; use OCP\Encryption\Exceptions\GenericEncryptionException; @@ -139,28 +141,36 @@ public function filesize($path) { $size = $this->unencryptedSize[$fullPath]; // update file cache if ($info instanceof ICacheEntry) { - $info = $info->getData(); $info['encrypted'] = $info['encryptedVersion']; } else { if (!is_array($info)) { $info = []; } $info['encrypted'] = true; + $info = new CacheEntry($info); } - $info['size'] = $size; - $this->getCache()->put($path, $info); + if ($size !== $info->getUnencryptedSize()) { + $this->getCache()->update($info->getId(), [ + 'unencrypted_size' => $size + ]); + } return $size; } if (isset($info['fileid']) && $info['encrypted']) { - return $this->verifyUnencryptedSize($path, $info['size']); + return $this->verifyUnencryptedSize($path, $info->getUnencryptedSize()); } return $this->storage->filesize($path); } + /** + * @param string $path + * @param array $data + * @return array + */ private function modifyMetaData(string $path, array $data): array { $fullPath = $this->getFullPath($path); $info = $this->getCache()->get($path); @@ -170,7 +180,7 @@ private function modifyMetaData(string $path, array $data): array { $data['size'] = $this->unencryptedSize[$fullPath]; } else { if (isset($info['fileid']) && $info['encrypted']) { - $data['size'] = $this->verifyUnencryptedSize($path, $info['size']); + $data['size'] = $this->verifyUnencryptedSize($path, $info->getUnencryptedSize()); $data['encrypted'] = true; } } @@ -478,7 +488,7 @@ public function fopen($path, $mode) { * * @return int unencrypted size */ - protected function verifyUnencryptedSize($path, $unencryptedSize) { + protected function verifyUnencryptedSize(string $path, int $unencryptedSize): int { $size = $this->storage->filesize($path); $result = $unencryptedSize; @@ -510,7 +520,7 @@ protected function verifyUnencryptedSize($path, $unencryptedSize) { * * @return int calculated unencrypted size */ - protected function fixUnencryptedSize($path, $size, $unencryptedSize) { + protected function fixUnencryptedSize(string $path, int $size, int $unencryptedSize): int { $headerSize = $this->getHeaderSize($path); $header = $this->getHeader($path); $encryptionModule = $this->getEncryptionModule($path); @@ -581,7 +591,9 @@ protected function fixUnencryptedSize($path, $size, $unencryptedSize) { $cache = $this->storage->getCache(); if ($cache) { $entry = $cache->get($path); - $cache->update($entry['fileid'], ['size' => $newUnencryptedSize]); + $cache->update($entry['fileid'], [ + 'unencrypted_size' => $newUnencryptedSize + ]); } return $newUnencryptedSize; @@ -621,7 +633,12 @@ private function fread_block($handle, int $blockSize): string { * @param bool $preserveMtime * @return bool */ - public function moveFromStorage(Storage\IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime = true) { + public function moveFromStorage( + Storage\IStorage $sourceStorage, + $sourceInternalPath, + $targetInternalPath, + $preserveMtime = true + ) { if ($sourceStorage === $this) { return $this->rename($sourceInternalPath, $targetInternalPath); } @@ -656,7 +673,13 @@ public function moveFromStorage(Storage\IStorage $sourceStorage, $sourceInternal * @param bool $isRename * @return bool */ - public function copyFromStorage(Storage\IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime = false, $isRename = false) { + public function copyFromStorage( + Storage\IStorage $sourceStorage, + $sourceInternalPath, + $targetInternalPath, + $preserveMtime = false, + $isRename = false + ) { // TODO clean this up once the underlying moveFromStorage in OC\Files\Storage\Wrapper\Common is fixed: // - call $this->storage->copyFromStorage() instead of $this->copyBetweenStorage @@ -676,7 +699,13 @@ public function copyFromStorage(Storage\IStorage $sourceStorage, $sourceInternal * @param bool $isRename * @param bool $keepEncryptionVersion */ - private function updateEncryptedVersion(Storage\IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath, $isRename, $keepEncryptionVersion) { + private function updateEncryptedVersion( + Storage\IStorage $sourceStorage, + $sourceInternalPath, + $targetInternalPath, + $isRename, + $keepEncryptionVersion + ) { $isEncrypted = $this->encryptionManager->isEnabled() && $this->shouldEncrypt($targetInternalPath); $cacheInformation = [ 'encrypted' => $isEncrypted, @@ -725,7 +754,13 @@ private function updateEncryptedVersion(Storage\IStorage $sourceStorage, $source * @return bool * @throws \Exception */ - private function copyBetweenStorage(Storage\IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime, $isRename) { + private function copyBetweenStorage( + Storage\IStorage $sourceStorage, + $sourceInternalPath, + $targetInternalPath, + $preserveMtime, + $isRename + ) { // for versions we have nothing to do, because versions should always use the // key from the original file. Just create a 1:1 copy and done @@ -743,7 +778,7 @@ private function copyBetweenStorage(Storage\IStorage $sourceStorage, $sourceInte if (isset($info['encrypted']) && $info['encrypted'] === true) { $this->updateUnencryptedSize( $this->getFullPath($targetInternalPath), - $info['size'] + $info->getUnencryptedSize() ); } $this->updateEncryptedVersion($sourceStorage, $sourceInternalPath, $targetInternalPath, $isRename, true); @@ -808,13 +843,6 @@ private function copyBetweenStorage(Storage\IStorage $sourceStorage, $sourceInte return (bool)$result; } - /** - * get the path to a local version of the file. - * The local version of the file can be temporary and doesn't have to be persistent across requests - * - * @param string $path - * @return string - */ public function getLocalFile($path) { if ($this->encryptionManager->isEnabled()) { $cachedFile = $this->getCachedFile($path); @@ -825,11 +853,6 @@ public function getLocalFile($path) { return $this->storage->getLocalFile($path); } - /** - * Returns the wrapped storage's value for isLocal() - * - * @return bool wrapped storage's isLocal() value - */ public function isLocal() { if ($this->encryptionManager->isEnabled()) { return false; @@ -837,15 +860,11 @@ public function isLocal() { return $this->storage->isLocal(); } - /** - * see https://www.php.net/manual/en/function.stat.php - * only the following keys are required in the result: size and mtime - * - * @param string $path - * @return array - */ public function stat($path) { $stat = $this->storage->stat($path); + if (!$stat) { + return false; + } $fileSize = $this->filesize($path); $stat['size'] = $fileSize; $stat[7] = $fileSize; @@ -853,14 +872,6 @@ public function stat($path) { return $stat; } - /** - * see https://www.php.net/manual/en/function.hash.php - * - * @param string $type - * @param string $path - * @param bool $raw - * @return string - */ public function hash($type, $path, $raw = false) { $fh = $this->fopen($path, 'rb'); $ctx = hash_init($type); @@ -1068,6 +1079,13 @@ public function writeStream(string $path, $stream, int $size = null): int { [$count, $result] = \OC_Helper::streamCopy($stream, $target); fclose($stream); fclose($target); + + // object store, stores the size after write and doesn't update this during scan + // manually store the unencrypted size + if ($result && $this->getWrapperStorage()->instanceOfStorage(ObjectStoreStorage::class)) { + $this->getCache()->put($path, ['unencrypted_size' => $count]); + } + return $count; } } diff --git a/lib/public/Files/Cache/ICacheEntry.php b/lib/public/Files/Cache/ICacheEntry.php index 17eecf89ddb2f..e1e8129394c71 100644 --- a/lib/public/Files/Cache/ICacheEntry.php +++ b/lib/public/Files/Cache/ICacheEntry.php @@ -162,4 +162,14 @@ public function getCreationTime(): ?int; * @since 18.0.0 */ public function getUploadTime(): ?int; + + /** + * Get the unencrypted size + * + * This might be different from the result of getSize + * + * @return int + * @since 25.0.0 + */ + public function getUnencryptedSize(): int; } diff --git a/tests/lib/Files/ObjectStore/S3Test.php b/tests/lib/Files/ObjectStore/S3Test.php index a7a95d5337576..fd451dc3c016f 100644 --- a/tests/lib/Files/ObjectStore/S3Test.php +++ b/tests/lib/Files/ObjectStore/S3Test.php @@ -176,7 +176,7 @@ public function testFileSizes($size) { // end of file reached fseek($result, $size); - self:self::assertTrue(feof($result)); + self::assertTrue(feof($result)); $this->assertNoUpload('testfilesizes'); } diff --git a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php index d26e5c499e70b..ebb97a25c77e0 100644 --- a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php +++ b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php @@ -5,6 +5,7 @@ use OC\Encryption\Exceptions\ModuleDoesNotExistsException; use OC\Encryption\Update; use OC\Encryption\Util; +use OC\Files\Cache\CacheEntry; use OC\Files\Storage\Temporary; use OC\Files\Storage\Wrapper\Encryption; use OC\Files\View; @@ -259,7 +260,7 @@ public function testGetMetaData($path, $metaData, $encrypted, $unencryptedSizeSe ->method('get') ->willReturnCallback( function ($path) use ($encrypted) { - return ['encrypted' => $encrypted, 'path' => $path, 'size' => 0, 'fileid' => 1]; + return new CacheEntry(['encrypted' => $encrypted, 'path' => $path, 'size' => 0, 'fileid' => 1]); } ); @@ -332,7 +333,7 @@ public function testFilesize() { ->disableOriginalConstructor()->getMock(); $cache->expects($this->any()) ->method('get') - ->willReturn(['encrypted' => true, 'path' => '/test.txt', 'size' => 0, 'fileid' => 1]); + ->willReturn(new CacheEntry(['encrypted' => true, 'path' => '/test.txt', 'size' => 0, 'fileid' => 1])); $this->instance = $this->getMockBuilder('\OC\Files\Storage\Wrapper\Encryption') ->setConstructorArgs( @@ -910,7 +911,7 @@ public function testCopyBetweenStorageVersions($sourceInternalPath, $targetInter if ($copyResult) { $cache->expects($this->once())->method('get') ->with($sourceInternalPath) - ->willReturn(['encrypted' => $encrypted, 'size' => 42]); + ->willReturn(new CacheEntry(['encrypted' => $encrypted, 'size' => 42])); if ($encrypted) { $instance->expects($this->once())->method('updateUnencryptedSize') ->with($mountPoint . $targetInternalPath, 42); diff --git a/tests/lib/HelperStorageTest.php b/tests/lib/HelperStorageTest.php index 6d7ea513d3f49..d3f480502b2d8 100644 --- a/tests/lib/HelperStorageTest.php +++ b/tests/lib/HelperStorageTest.php @@ -104,6 +104,9 @@ public function testGetStorageInfoExcludingExtStorage() { $extStorage->file_put_contents('extfile.txt', 'abcdefghijklmnopq'); $extStorage->getScanner()->scan(''); // update root size + $config = \OC::$server->getConfig(); + $config->setSystemValue('quota_include_external_storage', false); + \OC\Files\Filesystem::mount($extStorage, [], '/' . $this->user . '/files/ext'); $storageInfo = \OC_Helper::getStorageInfo(''); From de63f6363f1ae590c9735fbe9592835c04ab32cd Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 16 Aug 2022 11:16:14 +0200 Subject: [PATCH 5/5] fix updating size when folder is empty Signed-off-by: Robin Appelman --- lib/private/Files/Cache/Cache.php | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php index 5100889637332..ec284282178e9 100644 --- a/lib/private/Files/Cache/Cache.php +++ b/lib/private/Files/Cache/Cache.php @@ -891,7 +891,7 @@ public function calculateFolderSize($path, $entry = null) { return (int)$row['unencrypted_size']; }, $rows); $unencryptedSizes = array_map(function (array $row) { - return (int)(($row['unencrypted_size'] > 0) ? $row['unencrypted_size']: $row['size']); + return (int)(($row['unencrypted_size'] > 0) ? $row['unencrypted_size'] : $row['size']); }, $rows); $sum = array_sum($sizes); @@ -913,18 +913,22 @@ public function calculateFolderSize($path, $entry = null) { } else { $unencryptedTotal = $unencryptedSum; } - if ($entry['size'] !== $totalSize) { - // only set unencrypted size for a folder if any child entries have it set - if ($unencryptedMax > 0) { - $this->update($id, [ - 'size' => $totalSize, - 'unencrypted_size' => $unencryptedTotal, - ]); - } else { - $this->update($id, [ - 'size' => $totalSize, - ]); - } + } else { + $totalSize = 0; + $unencryptedTotal = 0; + $unencryptedMax = 0; + } + if ($entry['size'] !== $totalSize) { + // only set unencrypted size for a folder if any child entries have it set, or the folder is empty + if ($unencryptedMax > 0 || $totalSize === 0) { + $this->update($id, [ + 'size' => $totalSize, + 'unencrypted_size' => $unencryptedTotal, + ]); + } else { + $this->update($id, [ + 'size' => $totalSize, + ]); } } }