Skip to content
New issue

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

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

Already on GitHub? # to your account

Use mimetype from cache for workflow checks #25768

Merged
merged 3 commits into from
Dec 20, 2022

Conversation

icewind1991
Copy link
Member

  • Use the mimetype out of the cache instead of re-calculating it
  • Only detect mimetype from content for local storages
  • Remove redundant file_exists check
  • Add some tests

Motivation is that when using object stores, the file is added to the cache (and thus triggering workflow hooks) before it's written to the storage.

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Feb 23, 2021
@icewind1991 icewind1991 added this to the Nextcloud 22 milestone Feb 23, 2021
@icewind1991 icewind1991 force-pushed the workflow-use-cache-for-mimetype branch from 264c449 to de77189 Compare February 23, 2021 18:21
@rullzer rullzer requested a review from LukasReschke March 4, 2021 20:28
@nickvergessen nickvergessen requested review from blizzz and removed request for nickvergessen March 4, 2021 21:47
@blizzz
Copy link
Member

blizzz commented Mar 4, 2021

Only detect mimetype from content for local storages

That renders the check fairly useless when working with non-local files, doesn't it?

@icewind1991
Copy link
Member Author

I'm unsure about having the content-based detection in the first place, having the filecache and workflow both use different sources of mimetypes is bound to lead to confusion

This was referenced May 20, 2021
@blizzz blizzz mentioned this pull request Jun 2, 2021
57 tasks
@MorrisJobke MorrisJobke mentioned this pull request Jun 10, 2021
59 tasks
This was referenced Jun 16, 2021
@blizzz
Copy link
Member

blizzz commented Jun 23, 2021

CI is unhappy

@blizzz blizzz modified the milestones: Nextcloud 22, Nextcloud 23 Jun 24, 2021
@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Aug 18, 2021
@skjnldsv skjnldsv mentioned this pull request Oct 13, 2021
@skjnldsv
Copy link
Member

/rebase

@skjnldsv skjnldsv modified the milestones: Nextcloud 23, Nextcloud 24 Oct 21, 2021
@nextcloud-command nextcloud-command force-pushed the workflow-use-cache-for-mimetype branch from de77189 to 919e256 Compare October 21, 2021 15:53
@skjnldsv skjnldsv mentioned this pull request Mar 24, 2022
@blizzz blizzz mentioned this pull request Mar 31, 2022
@blizzz blizzz mentioned this pull request Apr 13, 2022
@blizzz blizzz modified the milestones: Nextcloud 24, Nextcloud 25 Apr 21, 2022
This was referenced Aug 12, 2022
This was referenced Aug 24, 2022
This was referenced Sep 6, 2022
@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
This was referenced Sep 20, 2022
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
Signed-off-by: Robin Appelman <robin@icewind.nl>
the method is only called if the file exists already

a check against storing the fallback mimetype is added as a safety instead

Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
@PVince81 PVince81 force-pushed the workflow-use-cache-for-mimetype branch from 919e256 to cb4648d Compare December 20, 2022 12:29
@PVince81
Copy link
Member

rebased and conflict solved

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

I wasn't able to run into such situation myself with minio as primary object store, however I sent this to someone having issues with flow and mime types and can confirm that this fixed the problem. Let's ship this!

@PVince81
Copy link
Member

/backport to stable25

@PVince81
Copy link
Member

/backport to stable24

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants