Conversation
- Add introductionVersion tags to all new config fields - Fix %w error wrapping in statFile (was using %s/bare string) - Remove dead resolveStatPath function - Invert ThumbnailBackend switch so default creates gRPC client (unknown values now fall through to gRPC, matching spec intent) - Extract shouldUseImagor() method to eliminate duplicated dispatch guard - Fix parseMaxInputFileSize: use TrimSuffix instead of TrimRight (character-set trimming broke MB/KB suffixes)
- Add ThumbnailResolutions config option to support configurable target resolutions - Implement ClosestMatch resolution in imagor handler using shared thumbnail package - Add deprecation warning for DisablePreviews flag at service startup - Fix parseMaxInputFileSize case sensitivity bug for GB/MB/KB suffixes - Add unit tests for size parsing and integration tests for imagor endpoint
- Remove redundant sourceSize assignment that always equaled requestedRes - Replace six-case switch in parseMaxInputFileSize with lookup table
…violations - Derive Content-Type in S3Cache.Put() using mime.TypeByExtension(key) instead of hardcoding application/octet-stream - Inline credentials.NewStaticV4 calls in test cases per Effective Go - Add tests for Content-Type derivation (.jpg, .png, unknown extension)
- Add S3 cache env vars to config: BUCKET, REGION, ENDPOINT, ACCESS_KEY, SECRET_KEY (WEBDAV_THUMBNAIL_CACHE_S3_* prefix) - Update NewThumbnailCache factory to accept *S3CacheConfig parameter - Add 's3' case that creates S3Cache when config is complete, NoopCache otherwise - Add BuildS3CacheConfig helper to construct S3CacheConfig from raw env var fields - Log startup warning when imagor + s3 cache configured but S3 config incomplete - Add unit tests for factory 's3' case and BuildS3CacheConfig
Implement FileCache as a filesystem-based thumbnail cache backend. When
WEBDAV_THUMBNAIL_CACHE_BACKEND=file, thumbnails are stored on disk using
hierarchical directories (checksum[:2]/checksum[2:4]/checksum[4:]/{w}x{h}.{ext})
for filesystem efficiency and atomic writes via temp file + rename pattern.
- FileCache implements ThumbnailCache interface with Get/Put methods
- Hierarchical directory layout mirrors existing thumbnail service storage
- Atomic writes prevent partial files on crash (CreateTemp -> Write -> Sync -> Rename)
- Directory creation with 0700 permissions, parent dirs created as needed
- Put is idempotent — no-op if file already exists for multi-instance safety
- Configured via ThumbnailCacheBackend=file and ThumbnailCacheDir env var
- Default cache directory: /var/lib/opencloud/webdav/thumbnails
- Comprehensive unit tests covering Get/Put/miss, atomic writes, idempotency
- Add resolvePublicLinkAuth to authenticate via gateway with type 'publicshares'
- Handle pre-signed public links with signature+expiration query params
- Build stat path as /public/{token}/{filepath} for public links
- Dispatch PublicThumbnail and PublicThumbnailHead to imagor flow when configured
- Cache keys use 'public-' prefix to avoid collisions with authenticated thumbnails
- Refactor imagorThumbnail/imagorThumbnailPublic into shared imagorThumbnailWithPrefix
- Add tests: successful thumbnail, pre-signed links, password rejection, expired tokens, HEAD checks
- Extract resolveThumbnailBackend for testable backend wiring logic - Add 8 tests for ThumbnailBackend values (grpc, imagor, none) - Add 5 tests for env var fallback with semicolon separator pattern - Add 6 tests for shared extensionInfo map - Extract shared ext->contentType map from outputExtension/extensionToContentType - Fix stale config description to include 'file' cache backend - Return value instead of pointer in getExtensionInfo (safety) - Add proper doc comments on exported struct fields
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Add a stateless POST endpoint that accepts raw image bytes via multipart
form and returns resized thumbnail bytes. Mimics imagor's /unsafe/ API
so webdav can use the same client code for both built-in generator and
external imagor instances.
Routes:
POST /unsafe/fit-in/{W}x{H}/filters:no_upscale()/filters:format({ext})/
POST /unsafe/{W}x{H}/filters:format({ext})/
Supports jpeg, png, gif output formats. Fit-in mode preserves aspect ratio
with no-upscale behavior. Fill mode crops to exact dimensions.
The push handler now respects the enable_vips build flag like the rest of the thumbnails service. Common logic (route parsing, encoding, error handling) lives in push.go. Backend-specific image processing is in: push_imaging.go (!enable_vips): imaging.Decode + Fit/Fill push_vips.go (enable_vips): vips.NewImageFromBuffer + ThumbnailWithSize Encoding uses stdlib for all formats since libvips does not support GIF output. The vips backend converts back to image.Image via PNG export so stdlib encoders can handle jpg/png/gif uniformly.
Prefactoring for the unified thumbnail architecture. Moves the preprocessor package from the thumbnails service to webdav where it will be used by the new ThumbnailWorkflow. Copies mime type and resolution matching utilities so webdav can validate files independently. - Move services/thumbnails/pkg/preprocessor/ → services/webdav/pkg/preprocessor/ with local error vars (replaces cross-service import) - Copy SupportedMimeTypes maps + IsMimeTypeSupported to webdav thumbnail pkg - Copy ParseResolutions + ClosestMatch to webdav thumbnail pkg - Update two imports in thumbnails service to point to new location No behavioral changes — same code, different location.
butonic
added
Type:Maintenance
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 5 critical |
| CodeStyle | 6 minor |
🟢 Metrics 515 complexity · 121 duplication
Metric Results Complexity 515 Duplication 121
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.
Rewrites webdav's thumbnail handling with a single ThumbnailWorkflow type that owns the complete pipeline (stat → validate → cache check → download → generate → cache → respond). This eliminates the strategy pattern and the backend enum config. - Rename pkg/imagor/ → pkg/generator/ (PostToImagor→PostImage, BuildImagorURL→BuildURL) - Create ThumbnailWorkflow in pkg/thumbnail/workflow/ with injected dependencies (Stater, FileDownloader, cache, *http.Client) - Delete Thumbnailer interface + imagorThumbnailer + grpcThumbnailer - New config: ThumbnailGeneratorURL, ThumbnailGeneratorTimeout, ThumbnailGeneratorAuthHeader, MaxInputFileSize - Remove old config: DisablePreviews, ThumbnailBackend, ImagorURL, ImagorTimeout, ImagorMaxInputFileSize - All four handlers delegate to workflow.Execute/ExecutePublic/Head - Integration tests moved to workflow package (12 specs)
After webdav took over the complete thumbnail workflow (slices 01-03), the thumbnails service is now a stateless image resizer. Delete all pull-based gRPC path code that is no longer used. Deleted: - Proto files + generated Go code (thumbnails service + messages) - gRPC service handler (pkg/service/grpc/) with decorators - JWT transfer token code (pkg/service/jwt/) - Storage layer (pkg/thumbnail/storage/) - filesystem storage, keys - Image source fetching (pkg/thumbnail/imgsource/) - webdav + CS3 - Duplicated utilities (encoding, generators, processor, resolutions) - /data HTTP endpoint + TransferTokenValidator middleware - gRPC server (pkg/server/grpc/) Kept: - Push endpoint (push.go, push_imaging.go, push_vips.go) - Minimal SupportedMimeTypes map (still imported by reva dependency) - HTTP server, debug server, config, metrics Repointed webdav search + graph sharedbyme/sharedwithme to use webdav's own copy of SupportedMimeTypes.
Thumbnails service is now a stateless image resizer; remove all leftover code from the old pull-based architecture: - Delete pkg/errors (error types belonged to deleted generators/imgsource) - Delete pkg/config/grpc.go, GRPC config field, debug grpc check - Delete dead Thumbnail config fields + FileSystemStorage type - Delete unused GRPCClientTLS/GrpcClient and go-micro client creation - Delete HTTP.Root (never read) - Reduce metrics to BuildInfo only; delete no-op instrument/logging wrappers - Remove unused Thumbnails.config field from HTTP service - Delete testdata files, Makefile protobuf target - Rewrite README for the stateless push endpoint Webdav: - Remove unused Webdav.cache struct field - Remove dead errTooEarly/errTooManyRequests/addRetryAfterHeader helpers - Remove dead generator.PostImage duplicate (workflow has its own) Opencloud init: - Remove thumbnails transfer_secret generation and config section
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.