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

implements search on null/notnull metadata #41459

Merged
merged 5 commits into from
Nov 23, 2023

Conversation

ArtificialOwl
Copy link
Member

@ArtificialOwl ArtificialOwl commented Nov 14, 2023

Add the possiblity to search on null/not null value on metadata.
This is a test-feature for the Photos App.

This PR requires to add this line

      '{DAV:}is-defined'    => Operator::class,

in https://github.com/nextcloud/3rdparty/blob/master/icewind/searchdav/src/DAV/QueryParser.php#L78

[...]
     <d:where>
        <d:and>
          <d:not>
          <d:is-defined>
            <d:prop><nc:metadata-test/></d:prop>
            </d:is-defined>
          </d:not>
        </d:and>
[...]

Need:

Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Nice

lib/private/Files/Cache/SearchBuilder.php Outdated Show resolved Hide resolved
apps/dav/lib/Files/FileSearchBackend.php Outdated Show resolved Hide resolved
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/search-metadata-null branch from b77e16c to 5229965 Compare November 14, 2023 22:14
@ArtificialOwl ArtificialOwl added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 14, 2023
@icewind1991
Copy link
Member

might be cleaner to just transform the dav operator to field == null !(field == null) instead of adding a new search operator

@ArtificialOwl ArtificialOwl force-pushed the enh/noid/search-metadata-null branch from 5229965 to a091bf0 Compare November 14, 2023 22:43
@artonge
Copy link
Contributor

artonge commented Nov 15, 2023

might be cleaner to just transform the dav operator to field == null !(field == null) instead of adding a new search operator

Do you mean in the DAV request ? The issue is that we are casting the type of the property here:

private function castValue(SearchPropertyDefinition $property, $value) {
switch ($property->dataType) {
case SearchPropertyDefinition::DATATYPE_BOOLEAN:
return $value === 'yes';
case SearchPropertyDefinition::DATATYPE_DECIMAL:
case SearchPropertyDefinition::DATATYPE_INTEGER:
case SearchPropertyDefinition::DATATYPE_NONNEGATIVE_INTEGER:
return 0 + $value;
case SearchPropertyDefinition::DATATYPE_DATETIME:
if (is_numeric($value)) {
return max(0, 0 + $value);
}
$date = \DateTime::createFromFormat(\DateTimeInterface::ATOM, (string)$value);
return ($date instanceof \DateTime && $date->getTimestamp() !== false) ? $date->getTimestamp() : 0;
default:
return $value;
}
}

And as the property is used in another comparison in the request, it is assumed to be an integer. So using the appropriate operator would be cleaner I think.

@ArtificialOwl ArtificialOwl force-pushed the enh/noid/search-metadata-null branch from a091bf0 to ab03a38 Compare November 15, 2023 21:03
@artonge
Copy link
Contributor

artonge commented Nov 16, 2023

Pushed 3rdparty changes

@ArtificialOwl ArtificialOwl force-pushed the enh/noid/search-metadata-null branch from cf5b620 to 1386738 Compare November 21, 2023 15:18
@solracsf solracsf added this to the Nextcloud 28 milestone Nov 21, 2023
@blizzz blizzz mentioned this pull request Nov 22, 2023
5 tasks
ArtificialOwl and others added 4 commits November 22, 2023 12:43
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
Signed-off-by: Louis Chemineau <louis@chmn.me>
Signed-off-by: Louis Chemineau <louis@chmn.me>
Signed-off-by: Louis Chemineau <louis@chmn.me>
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/search-metadata-null branch from 1386738 to a93af13 Compare November 22, 2023 13:43
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

👍 for the caldav changes

@ArtificialOwl
Copy link
Member Author

Working on fixing test.
Since commit ..., the array returned in the response is reordered:

  • "name": "{DAV:}href",
  • "name": "{DAV:}propstat",
  • "name": "{DAV:}status",

(status was before propstat before)

@ArtificialOwl ArtificialOwl force-pushed the enh/noid/search-metadata-null branch 3 times, most recently from 8d06f48 to a1d2ac5 Compare November 23, 2023 01:23
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/search-metadata-null branch from a1d2ac5 to fbe92d4 Compare November 23, 2023 01:43
@ArtificialOwl ArtificialOwl merged commit ee787cd into master Nov 23, 2023
50 checks passed
@ArtificialOwl ArtificialOwl deleted the enh/noid/search-metadata-null branch November 23, 2023 08:52
@ChristophWurst ChristophWurst added the pending documentation This pull request needs an associated documentation update label Nov 23, 2023
@ChristophWurst
Copy link
Member

adds new public API -> needs docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews pending documentation This pull request needs an associated documentation update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants