Skip to content

Empty values operators #258

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Closed
fogelito wants to merge 8 commits into from
Closed

Empty values operators #258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1bff401
Empty values operators
fogelito Apr 10, 2023
5ef3992
formatting and composer.lock
fogelito Apr 10, 2023
c79ab47
formatting and throws
fogelito Apr 10, 2023
5b8afbd
Update src/Database/Query.php
fogelito Apr 10, 2023
2ccebc9
Update src/Database/Query.php
fogelito Apr 10, 2023
bf8e8d0
Text fixes
fogelito Apr 10, 2023
8d62347
Text fixes #2
fogelito Apr 10, 2023
2e56e84
test in internal $id as well
fogelito Apr 10, 2023
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
2 changes: 1 addition & 1 deletion composer.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 38 additions & 1 deletion src/Database/Query.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,9 +482,13 @@ protected static function getMethodFromAlias(string $method): string
* @param string $attribute
* @param array<mixed> $values
* @return Query
* @throws Exception
*/
public static function equal(string $attribute, array $values): self
{
if (empty($values)) {
throw new Exception('Equal queries require at least one value.');
Copy link
Contributor

@Meldiron Meldiron Apr 11, 2023

Choose a reason for hiding this comment

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

Should we throw error, or continue without a query?
As Appwrite developer, if I do [Query.equal('userId', ids]) and ids are empty, it would be annoying to get an error. It would mean I need to do exactly the same check in MANY places - everywhere, where I put array into query.

To keep SDK and HTTP consistent, I would avoid fixing this in SDK. I think this should be fixed here, or in Appwrite repo. I think empty array of values should return everything, instead of throwing error.

Copy link
Contributor Author

@fogelito fogelito Apr 11, 2023

Choose a reason for hiding this comment

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

Did not realize the SDK fixing.. We can do it with a validator if we want to throw an error
The question is Should we throw errors, or select all? especially now that we removed limits?
@abnegate what do you think

Copy link
Member

@abnegate abnegate Apr 11, 2023

Choose a reason for hiding this comment

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

I think we should throw an error. In that way, the developer knows the query has an issue. They might not realize it's not doing anything if we just ignore it. Make more sense to do it in the validator though instead of here

}
return new self(self::TYPE_EQUAL, $attribute, $values);
}

Expand All @@ -494,9 +498,13 @@ public static function equal(string $attribute, array $values): self
* @param string $attribute
* @param mixed $value
* @return Query
* @throws Exception
*/
public static function notEqual(string $attribute, mixed $value): self
{
if ($value === null) {
throw new Exception('Not equal queries cannot have a null value, Please use Is not null operator.');
}
return new self(self::TYPE_NOTEQUAL, $attribute, [$value]);
}

Expand All @@ -506,9 +514,13 @@ public static function notEqual(string $attribute, mixed $value): self
* @param string $attribute
* @param mixed $value
* @return Query
* @throws Exception
*/
public static function lessThan(string $attribute, mixed $value): self
{
if ($value === null) {
throw new Exception('Less than queries cannot have a null value.');
}
return new self(self::TYPE_LESSER, $attribute, [$value]);
}

Expand All @@ -518,9 +530,13 @@ public static function lessThan(string $attribute, mixed $value): self
* @param string $attribute
* @param mixed $value
* @return Query
* @throws Exception
*/
public static function lessThanEqual(string $attribute, mixed $value): self
{
if ($value === null) {
throw new Exception('Less than equal queries cannot have a null value.');
}
return new self(self::TYPE_LESSEREQUAL, $attribute, [$value]);
}

Expand All @@ -530,9 +546,13 @@ public static function lessThanEqual(string $attribute, mixed $value): self
* @param string $attribute
* @param mixed $value
* @return Query
* @throws Exception
*/
public static function greaterThan(string $attribute, mixed $value): self
{
if ($value === null) {
throw new Exception('Greater than queries cannot have a null value.');
}
return new self(self::TYPE_GREATER, $attribute, [$value]);
}

Expand All @@ -542,9 +562,13 @@ public static function greaterThan(string $attribute, mixed $value): self
* @param string $attribute
* @param mixed $value
* @return Query
* @throws Exception
*/
public static function greaterThanEqual(string $attribute, mixed$value): self
public static function greaterThanEqual(string $attribute, mixed $value): self
{
if ($value === null) {
throw new Exception('Greater than equal queries cannot have a null value.');
}
return new self(self::TYPE_GREATEREQUAL, $attribute, [$value]);
}

Expand All @@ -554,9 +578,13 @@ public static function greaterThanEqual(string $attribute, mixed$value): self
* @param string $attribute
* @param array<mixed> $values
* @return Query
* @throws Exception
*/
public static function contains(string $attribute, array $values): self
{
if (empty($values)) {
throw new Exception('Contains queries require at least one value.');
}
return new self(self::TYPE_CONTAINS, $attribute, $values);
}

Expand All @@ -579,9 +607,18 @@ public static function between(string $attribute, mixed $start, mixed $end): sel
* @param string $attribute
* @param mixed $value
* @return Query
* @throws Exception
*/
public static function search(string $attribute, mixed $value): self
{
if ($value === null) {
throw new Exception('Search queries cannot have a null value.');
}

if ($value === '') {
throw new Exception('Search queries cannot have an empty value.');
}

return new self(self::TYPE_SEARCH, $attribute, [$value]);
}

Expand Down
102 changes: 102 additions & 0 deletions tests/Database/Base.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,108 @@ public function testFulltextIndexWithInteger(): void
static::getDatabase()->createIndex('documents', 'fulltext_integer', Database::INDEX_FULLTEXT, ['string','integer']);
}

public function testEmptyOperatorValues(): void
{
try {
static::getDatabase()->findOne('documents', [
Query::equal('string', []),
]);
$this->fail('Failed to throw exception');
} catch (Exception $e) {
$this->assertInstanceOf(Exception::class, $e);
$this->assertEquals('Equal queries require at least one value.', $e->getMessage());
}

try {
static::getDatabase()->findOne('documents', [
Query::search('$id', null),
]);
$this->fail('Failed to throw exception');
} catch (Exception $e) {
$this->assertInstanceOf(Exception::class, $e);
$this->assertEquals('Search queries cannot have a null value.', $e->getMessage());
}

try {
static::getDatabase()->findOne('documents', [
Query::search('string', ''),
]);
$this->fail('Failed to throw exception');
} catch (Exception $e) {
$this->assertInstanceOf(Exception::class, $e);
$this->assertEquals('Search queries cannot have an empty value.', $e->getMessage());
}

try {
static::getDatabase()->findOne('documents', [
Query::search('string', null),
]);
$this->fail('Failed to throw exception');
} catch (Exception $e) {
$this->assertInstanceOf(Exception::class, $e);
$this->assertEquals('Search queries cannot have a null value.', $e->getMessage());
}

try {
static::getDatabase()->findOne('documents', [
Query::notEqual('string', null),
]);
$this->fail('Failed to throw exception');
} catch (Exception $e) {
$this->assertInstanceOf(Exception::class, $e);
$this->assertEquals('Not equal queries cannot have a null value, Please use Is not null operator.', $e->getMessage());
}

try {
static::getDatabase()->findOne('documents', [
Query::lessThan('string', null),
]);
$this->fail('Failed to throw exception');
} catch (Exception $e) {
$this->assertInstanceOf(Exception::class, $e);
$this->assertEquals('Less than queries cannot have a null value.', $e->getMessage());
}

try {
static::getDatabase()->findOne('documents', [
Query::lessThanEqual('string', null),
]);
$this->fail('Failed to throw exception');
} catch (Exception $e) {
$this->assertInstanceOf(Exception::class, $e);
$this->assertEquals('Less than equal queries cannot have a null value.', $e->getMessage());
}

try {
static::getDatabase()->findOne('documents', [
Query::greaterThan('string', null),
]);
$this->fail('Failed to throw exception');
} catch (Exception $e) {
$this->assertInstanceOf(Exception::class, $e);
$this->assertEquals('Greater than queries cannot have a null value.', $e->getMessage());
}

try {
static::getDatabase()->findOne('documents', [
Query::greaterThanEqual('string', null),
]);
$this->fail('Failed to throw exception');
} catch (Exception $e) {
$this->assertInstanceOf(Exception::class, $e);
$this->assertEquals('Greater than equal queries cannot have a null value.', $e->getMessage());
}

try {
static::getDatabase()->findOne('documents', [
Query::contains('string', []),
]);
$this->fail('Failed to throw exception');
} catch (Exception $e) {
$this->assertInstanceOf(Exception::class, $e);
$this->assertEquals('Contains queries require at least one value.', $e->getMessage());
}
}

/**
* @depends testCreateDocument
Expand Down