Skip to content

add named metadata event - #41601

Merged
ArtificialOwl merged 1 commit into
masterfrom
enh/noid/named-metadata-event
Nov 20, 2023
Merged

add named metadata event#41601
ArtificialOwl merged 1 commit into
masterfrom
enh/noid/named-metadata-event

Conversation

@ArtificialOwl

Copy link
Copy Markdown
Member

This adds:

  • a new event MetadataNamedEvent,
  • an optional parameter to the existing occ files:scan --generate-metadata [name] command.

The idea is to provide a last param to the IFilesMetadataManager::refreshMetadata() that assign a name to the event.
Any app listening to the MetadataNameEvent will use getName() to confirm the event is related to itself and manage the refresh of metadata the same way it would manage the Live and/or Background Event.

This way, we can ask a refresh of the metadata from all files' from the cloud without loosing time refreshing all metadata provided by all apps.

@ArtificialOwlArtificialOwl added the 3. to review Waiting for reviews label Nov 18, 2023
@ArtificialOwlArtificialOwl added this to the Nextcloud 28 milestone Nov 18, 2023
@artonge

Copy link
Copy Markdown
Collaborator

So existing providers need to all listen to this new event?

if ((self::PROCESS_LIVE & $process) !== 0) {
$event = new MetadataLiveEvent($node, $metadata);
} elseif ((self::PROCESS_NAMED & $process) !== 0 && $namedEvent !== '') {
$event = new MetadataNamedEvent($node, $metadata, $namedEvent);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So named event is always live, or always backgrounded?
Why the name is not an optional parameter for both MetadataLiveEvent and MetadataBackgroundEvent?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Because it would require a check condition on every Live/Background listeners

@ArtificialOwl

Copy link
Copy Markdown
MemberAuthor

So existing providers need to all listen to this new event?

no, this is purely optionnal; only if they want to called via a specific occ command.
It should only be implemented if they want to provide a faster way to only add their own metadata on a scan

@come-nc

Copy link
Copy Markdown
Contributor

So the passed name does not need to match a metadata name, it’s like a tag used by some providers?

@ArtificialOwl

Copy link
Copy Markdown
MemberAuthor

So the passed name does not need to match a metadata name, it’s like a tag used by some providers?

yes, as we don't communicate via provider we cannot enforce provider_id

@ArtificialOwl

ArtificialOwl commented Nov 20, 2023

Copy link
Copy Markdown
MemberAuthor

The idea behind this is mostly to remove the custom files scan from files_fulltextsearch and use metadata to store the status of the index of a document there.

However, starting a full index of your current document will also trigger some heavy process from the extraction of metadata from photos.

Using NamedEvent, I can trigger a scan of files to only update metadata related to fulltextsearch

@come-nccome-nc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I get the idea, I’m not entirely sure this won’t be cause for trouble.
Especially there is nothing to prevent name clash between applications.

Comment threadapps/files/lib/Command/Scan.php
Comment threadlib/private/FilesMetadata/FilesMetadataManager.php Outdated
Comment threadlib/public/FilesMetadata/Event/MetadataNamedEvent.php Outdated
@blizzzblizzz mentioned this pull request Nov 20, 2023
5 tasks
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
@ArtificialOwl
ArtificialOwlforce-pushed the enh/noid/named-metadata-event branch from f9ec06b to 22d6c8dCompareNovember 20, 2023 14:36

@artongeartonge left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Feels highly specific, but I do not see any other solution.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ArtificialOwl@artonge@come-nc