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

[stable10] Allow configuring thumbnails path #35131

Merged
merged 2 commits into from
May 10, 2019

Conversation

PVince81
Copy link
Contributor

Similar to "cache_path" but for thumbnails.
This was quickly copied from the CacheMountProvider

Why ? Because I tried to set this up on my server with symlinks and it says "following symlinks not allowed".

@PVince81 PVince81 added this to the development milestone Apr 30, 2019
@PVince81 PVince81 self-assigned this Apr 30, 2019
@PVince81
Copy link
Contributor Author

One reason I want a separate thumbnails folder is because I consider this like a cache and don't want this in backups. Also having it separate makes it easier to clean up.

@PVince81
Copy link
Contributor Author

Tested on my own server by patching on top of 10.2.0 RC1, works fine.

My thumbnails are stored in "/var/cache/thumbnails/$user" now.

@PVince81 PVince81 requested a review from mmattel April 30, 2019 19:06
@PVince81 PVince81 changed the title [stable10] Allow configuring thumbnails patAllow configuring thumbnails pathh [stable10] Allow configuring thumbnails path Apr 30, 2019
config/config.sample.php Outdated Show resolved Hide resolved
@PVince81
Copy link
Contributor Author

PVince81 commented May 2, 2019

  • update doc
  • add unit tests for both the cache mount provider and the new one

@PVince81 PVince81 force-pushed the stable10-configurable-thumbnails-path branch from f240bc1 to 2dadf07 Compare May 7, 2019 12:49
@codecov
Copy link

codecov bot commented May 7, 2019

Codecov Report

Merging #35131 into stable10 will decrease coverage by 19.5%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##             stable10   #35131       +/-   ##
===============================================
- Coverage       64.47%   44.97%   -19.51%     
===============================================
  Files            1288      116     -1172     
  Lines           76936    11565    -65371     
  Branches         1307     1307               
===============================================
- Hits            49606     5201    -44405     
+ Misses          26949     5983    -20966     
  Partials          381      381
Flag Coverage Δ Complexity Δ
#javascript 53.01% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 30.71% <ø> (-34.99%) 0 <ø> (-20070)
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/Storage/DAV.php 59.45% <0%> (-21.64%) 0% <0%> (ø)
apps/updatenotification/templates/admin.php
lib/private/Encryption/Keys/Storage.php
lib/private/App/CodeChecker/NodeVisitor.php
lib/private/RedisFactory.php
apps/dav/lib/Avatars/AvatarNode.php
...s/dav/appinfo/Migrations/Version20170202213905.php
apps/dav/lib/Upload/ChunkLocationProvider.php
apps/files/lib/AppInfo/Application.php
apps/systemtags/list.php
... and 1163 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee5f45e...2dadf07. Read the comment docs.

@codecov
Copy link

codecov bot commented May 7, 2019

Codecov Report

Merging #35131 into stable10 will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@              Coverage Diff               @@
##             stable10   #35131      +/-   ##
==============================================
+ Coverage       64.48%   64.49%   +<.01%     
- Complexity      20074    20078       +4     
==============================================
  Files            1288     1289       +1     
  Lines           76955    76966      +11     
  Branches         1307     1307              
==============================================
+ Hits            49626    49637      +11     
  Misses          26948    26948              
  Partials          381      381
Flag Coverage Δ Complexity Δ
#javascript 53.01% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 65.71% <100%> (ø) 20078 <4> (+4) ⬆️
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/Mount/PreviewsMountProvider.php 100% <100%> (ø) 4 <4> (?)
lib/private/Server.php 86.56% <100%> (+0.01%) 253 <0> (ø) ⬇️
lib/private/Files/Mount/CacheMountProvider.php 100% <100%> (ø) 4 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b4e0e9...c57b8b7. Read the comment docs.

@PVince81
Copy link
Contributor Author

PVince81 commented May 7, 2019

@mmattel I've adjusted the doc, please check.

I've now written unit tests for both the previews mount provider and cache mount provider.
Apparently the "$loader" wasn't properly passed in.

@DeepDiver1975 please review

Copy link
Member

@DeepDiver1975 DeepDiver1975 left a comment

Choose a reason for hiding this comment

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

Just one minor phpdoc issue - 👍

private $config;

/**
* ObjectStoreHomeMountProvider constructor.
Copy link
Member

Choose a reason for hiding this comment

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

ObjectStore ❓

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed now in both cache and preview providers

@PVince81 PVince81 force-pushed the stable10-configurable-thumbnails-path branch from 2dadf07 to 223e83c Compare May 7, 2019 13:04
* and `$user` will be substituted with the user id automatically.
*
* For example if "previews_path" is "/var/cache/owncloud/thumbnails" then for a logged in
* user "user1" the thumbnail path will be "/var/cache/owncloud/thumbnails/user1".
Copy link
Contributor

@mmattel mmattel May 7, 2019

Choose a reason for hiding this comment

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

Can you pls use back-ticks instead quotes
As you correctly do in the block above...

The text is fine, thanks for adding it 👍

@PVince81 PVince81 force-pushed the stable10-configurable-thumbnails-path branch from 223e83c to c57b8b7 Compare May 9, 2019 14:30
@PVince81
Copy link
Contributor Author

PVince81 commented May 9, 2019

all adjusted, including CI issues

@PVince81 PVince81 merged commit 1e67d80 into stable10 May 10, 2019
@PVince81 PVince81 deleted the stable10-configurable-thumbnails-path branch May 10, 2019 06:58
@PVince81
Copy link
Contributor Author

master: #35196

@davitol davitol mentioned this pull request Sep 3, 2019
13 tasks
@lock lock bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants