Conversation
butonic
added
Type:Maintenance
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 8 critical |
| CodeStyle | 6 minor |
🟢 Metrics 567 complexity · 161 duplication
Metric Results Complexity 567 Duplication 161
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
|
How do audio artwork, geogebra thumbnail and tiff preview extraction fit into this approach? Have you seen that I reworked the tiff extraction pr to use tika? I already upstreamed audio artwork extraction and tiff extraction to tika. Geogebra is still pending, but I expect it to be merged very soon as well. So I would like to see a preprocessing step for extraction of embedded pictures before they are handed over to imagor for resizing (with tika it would be push based and out of process too). I was a bit surprised to see that you are moving so much into the webdav service, my expectation would have been that you just replace the Decoder in the thumbnails service - can you elaborate a little why you chose this way more invasive approach? |
Add a stateless POST endpoint that accepts an original image as a
multipart upload and returns the resized thumbnail, mimicking imagor's
/unsafe/ API:
POST /unsafe/fit-in/{width}x{height}/filters:no_upscale()/filters:format({ext})
POST /unsafe/{width}x{height}/filters:format({ext})
The first form scales down to fit within the given dimensions, the
second fills them exactly. Supported output formats are jpg, png and
gif. Image processing is split by build tag: stdlib imaging by default,
libvips when built with -tags enable_vips. The endpoint is stateless:
no auth, no storage, no source fetching.
…rkflow webdav now drives the full pipeline: stat -> validate -> cache check -> download -> preprocess -> generate -> cache -> respond. - Space-scoped refs are anchored at the space ResourceId (not a path mount). - Downloads authenticate with the user's x-access-token. - Preprocessing (audio/geogebra/text/gif) is wired in; direct images pass through. - Generator URL defaults to the local thumbnails service.
The thumbnails service is now a stateless image resizer; remove everything the old gRPC-based pipeline needed: - proto definitions and generated code (service + messages) - gRPC server, handler and decorators - JWT transfer token code and the /data HTTP download endpoint - filesystem storage layer and source fetchers (webdav + CS3 imgsource) - duplicated encoding/generator/processor/resolution utilities - dead config fields (Thumbnail struct, GRPC config, unused go-micro client), no-op metrics/instrumentation wrappers, errors package, stale testdata and the Makefile protobuf target Also remove the now-meaningless thumbnails transfer secret generation from opencloud init.
We can move the workflow into webdav, because webdav already has everything it needs locally (the CS3 gateway client for stat + download, the auth context, the response writer), which lets the generator shrink to a stateless resizer (POST bytes in, resized bytes out) that's swappable with an external imagor via a single URL. The thumbnail generation process is owned, end-to-end, by webdav instead of logic split across two services and two round-trips. And we can use an external container to support HEIC thumbnails, just by changing the URL. And I really want to be able to reuse the thumbnailer like tika, because it now becomes stateless. |
Okay, alright. Works for me.
👍🏻
I'm fine with that, I just wanted you to be aware. No problem at all to port it. It was just important to me that extractors still have a place in the architecture and we don't need a magical one service that can handle all file types.
Fair enough, thanks!
That part was clear :) One more note: It might be the time to at least quickly think about the "honest hasPreview" implementation.
Thoughts? Do you see any problems with your drafted architecture? (I don't see any, just to be sure) |
2 participants
This PR tries to fix #1128 in a backwards compatible way.
Thumbnail generation used to live entirely in the thumbnails service behind a gRPC API: webdav called GetThumbnail, the service fetched the source from storage, preprocessed and generated the image, stored it on its own filesystem, and returned a JWT-signed URL that webdav had to follow with a second authenticated HTTP call just to get the bytes back. This branch inverts that: the thumbnails service becomes a stateless imagor-compatible resizer (one POST endpoint — image bytes in, resized bytes out; no auth, storage, or gRPC), and webdav owns the whole workflow via a single ThumbnailWorkflow type: stat via gateway → cache check → download source → preprocess → POST to generator → cache → respond.
Webdav gains what it needs to own that pipeline: the preprocessors (PDF→image, text→image, audio cover art) moved over from the thumbnails service, a new checksum-keyed thumbnail cache with memory/file/S3/noop backends, and a pkg/generator package that builds resizer URLs and posts multipart images. Config shrinks to one generator URL plus timeout, optional auth header (for an external imagor behind a proxy), and max input file size — the URL can point at the built-in thumbnails service or any imagor instance.
The old architecture is deleted from the thumbnails service: proto files, gRPC handler, JWT transfer tokens, filesystem storage, source fetchers, the /data endpoint, and duplicated utilities (~4,000 lines), plus leftover dead config, no-op metrics wrappers, and opencloud init's now-meaningless transfer-secret generation. Net diff: 102 files, +3,337/−4,336; thumbnail requests no longer need the second round-trip, and the resizer is trivially replaceable.
Related:
#630
#3364
#3332
opencloud-eu/reva#781
opencloud-eu/reva#773
Discussion:
https://github.com/orgs/opencloud-eu/discussions/2368
https://github.com/orgs/opencloud-eu/discussions/1090
@dschmidt This PR is not ready, but I want to bring your attention to this approach, which is why I am pushing this code now.