Feature/noid/imagecaching

[newhinton]

• Adds image caching to some providers
• Support for nextcloud 28

closes #123
closes #131
implements #115
implements #2
closes #7
closes #9

[EtienneHenger]

Is there an estimate as to when this will be merged into main? Asking as I am looking to update to NC28 and am wondering if it is worth the hassle to update this app via the terminal as opposed to waiting a bit and having it in the official app release...

[newhinton]

Generally speaking, this PR is feature-complete. However, since i wrote it over a couple of month, and it introduces some big changes, i want to do a full review before merging. That still might take a while. Also, any help is appreciated and will speed this up!

[EtienneHenger]

Thanks for the quick reply! I would be happy to help any way I can; so how can I help? Mind you I am a novice in PHP, JavaScript, and CSS...

[pikacchuu]

Hello, thanks for the work, I tested and it works for the login page but not when you are logged in.
How can I help you?

[mwinkens]

Thank you for your PR! unsplash currently prevents me from updating to nc28, so this PR is very welcomed. I just request a few tiny changes, in order to give you some feedback and make this ready for merge. Feel free to challenge some of my review points, since I am not maintainer of this project.

Unfortunately I shouldn't review your javascript files and I still need to

• test this in production

[newhinton]

@mwinkens

Thank you for your review! Your work is very appreciated!

I will try to work through your notes later, the linting-idea is a good one, and i'll do something about it!

[mwinkens]

@mwinkens

Thank you for your review! Your work is very appreciated!

I will try to work through your notes later, the linting-idea is a good one, and i'll do something about it!

I also updated your description, feel free to change it accordingly! I noticed you mentioned the closed issues also in the commits

[newhinton]

I have not introduced an dedicated linter, however, i used my ide's tools to reformat the files. I am a bit hesitant to use a tool, because it seems most depend on some package manager like compose, and at the moment this app does not use one because it is rather simple, dependency-wise.

But i have worked through your helpful annotations and a lot of that was helpful! Thank you for making that effort!

[mwinkens]

Hello, thanks for the work, I tested and it works for the login page but not when you are logged in. How can I help you?

@newhinton I tested this PR now manually and I have the same problem. Normally you have a(n) (un)splash background when logged in, this is mostly visible in the dashboard. I can add screenshots if you want.

Otherwise this looks good, you still have a few conversations open (see above), I closed the ones that are resolved. Thank you for fixing the linting issues 👍

Edit:
@pikacchuu I found out, that this is an admin setting, you can enable/disable splash for the login and dashboard. Go To AdminSettings - Design (bottom).

Sorry for the screenshot being german, but I couldn't be bothered

[mwinkens]

Also I managed to produce some exceptions with the UnsplashAPI provider (I didn't provide a valid token)

{"reqId":"3PdFVjay1aLCnS1Ix60x","level":3,"time":"2024-03-18T11:46:52+00:00","remoteAddr":"127.0.0.1","user":"nextcloud27","app":"index","method":"GET","url":"/index.php/apps/unsplash/api/dashboard.css","message":"/appdata_oc9arpx6pmoc/unsplash/UnsplashAPI","userAgent":"Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:123.0) Gecko/20100101 Firefox/123.0","version":"28.0.1.1","exception":{"Exception":"OCP\\Files\\NotFoundException","Message":"/appdata_oc9arpx6pmoc/unsplash/UnsplashAPI","Code":0,"Trace":[{"file":"/var/www/nextcloud28/lib/private/Files/Node/LazyFolder.php","line":161,"function":"get","class":"OC\\Files\\Node\\Root","type":"->"},{"file":"/var/www/nextcloud28/lib/private/Files/AppData/AppData.php","line":132,"function":"get","class":"OC\\Files\\Node\\LazyFolder","type":"->"},{"file":"/home/marvin/workspace/unsplash/lib/ProviderHandler/Provider.php","line":236,"function":"getFolder","class":"OC\\Files\\AppData\\AppData","type":"->"},{"file":"/home/marvin/workspace/unsplash/lib/Provider/UnsplashAPI.php","line":51,"function":"getImageFolder","class":"OCA\\Unsplash\\ProviderHandler\\Provider","type":"->"},{"file":"/home/marvin/workspace/unsplash/lib/Provider/UnsplashAPI.php","line":46,"function":"getMetadata","class":"OCA\\Unsplash\\Provider\\UnsplashAPI","type":"->"},{"file":"/home/marvin/workspace/unsplash/lib/Services/SettingsService.php","line":263,"function":"getCachedImageURL","class":"OCA\\Unsplash\\Provider\\UnsplashAPI","type":"->"},{"file":"/home/marvin/workspace/unsplash/lib/Controller/CssController.php","line":114,"function":"headerbackgroundLink","class":"OCA\\Unsplash\\Services\\SettingsService","type":"->"},{"file":"/home/marvin/workspace/unsplash/lib/Controller/CssController.php","line":88,"function":"innerCSS","class":"OCA\\Unsplash\\Controller\\CssController","type":"->"},{"file":"/home/marvin/workspace/unsplash/lib/Controller/CssController.php","line":58,"function":"mediaQuery","class":"OCA\\Unsplash\\Controller\\CssController","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Http/Dispatcher.php","line":230,"function":"dashboard","class":"OCA\\Unsplash\\Controller\\CssController","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Http/Dispatcher.php","line":137,"function":"executeController","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/App.php","line":184,"function":"dispatch","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->"},{"file":"/var/www/nextcloud28/lib/private/Route/Router.php","line":315,"function":"main","class":"OC\\AppFramework\\App","type":"::"},{"file":"/var/www/nextcloud28/lib/base.php","line":1069,"function":"match","class":"OC\\Route\\Router","type":"->"},{"file":"/var/www/nextcloud28/index.php","line":39,"function":"handleRequest","class":"OC","type":"::"}],"File":"/var/www/nextcloud28/lib/private/Files/Node/Root.php","Line":207,"message":"/appdata_oc9arpx6pmoc/unsplash/UnsplashAPI","exception":{},"CustomMessage":"/appdata_oc9arpx6pmoc/unsplash/UnsplashAPI"}}

The other providers seem to work fine, but also seem to fill the cache folder, which is funny because the WikiCommon provider fills the nature photos with cars

[newhinton]

@mwinkens

I found out, that this is an admin setting, you can enable/disable splash for the login and dashboard.

So there is no issue at all and that part work for you now?

Also I managed to produce some exceptions with the UnsplashAPI provider (I didn't provide a valid token)

Good catch!

The other providers seem to work fine, but also seem to fill the cache folder

How bad is it, do you have some numbers?

[mwinkens]

@newhinton

So there is no issue at all and that part work for you now?

No there isn't, this was just a configuration issue!

The other providers seem to work fine, but also seem to fill the cache folder

The number of images isn't the problem, but with different providers the image content changes. If I switch between providers, I fill the image cache with cars and nature photos. Maybe the image cache needs to be deleted on provider switch?

Otherwise this looks good! I noticed, that there is a card on the dashboard and login screen with image information, but unfortunately it isn't clickable, is this part of this PR?

[newhinton]

Otherwise this looks good! I noticed, that there is a card on the dashboard and login screen with image information, but unfortunately it isn't clickable, is this part of this PR?

It should be clickable, do you have javascript disabled? Could also be a cache problem, can you reload with caching disabled?

[JW-CH]

Love to see the progress on this, came back to this because v29 was released.

[mwinkens]

Otherwise this looks good! I noticed, that there is a card on the dashboard and login screen with image information, but unfortunately it isn't clickable, is this part of this PR?

It should be clickable, do you have javascript disabled? Could also be a cache problem, can you reload with caching disabled?

I disabled the cache and reloaded the page, but the problem persists. This button just redirects me from the dashboard to the dashboard

Imo this PR is good enough, I would merge and address issues in follow ups!

[CharlesDelorme]

I would like to help ? How can I install this on my NC28 ?

[mwinkens]

I would like to help ? How can I install this on my NC28 ?

clone PR into nextcloud apps directory under the unsplash directory, don't forget to change directory permission with chown, php occ app:enable unsplash

[CharlesDelorme]

I would like to help ? How can I install this on my NC28 ?

clone PR into nextcloud apps directory under the unsplash directory, don't forget to change directory permission with chown, php occ app:enable unsplash

Thank you ! I read the doc and I'll try that.

[CharlesDelorme]

Unsplash 3.0.0 installed on 28.0.6

(For those not familiar with Git like me, I downloaded https://github.com/nextcloud/unsplash/archive/refs/heads/feature/noid/imagecaching.zip, unzip it in /var/www/nextcloud/apps, renamed it to unsplash and "occ app:enable unsplash", thank to mwinkens)

Both login and dashboard work, tint included. "high viz" (I try to translate from french) options don't have an obvious effect and I didn't set a token. Changing keywords worked also. All sources (not tested unsplash api) work also.

Logs in nextcloud.log show several php error for filter_var "Constant FILTER_SANITIZE_STRING is deprecated at"
/var/www/nextcloud/apps/unsplash/lib/Controller/AdminSettingsController.php#60 #62 #97

At my very small level, it looks like a go :-)