Repository navigation
feat: store filesizes for each FileResource - #2089
Merged
Merged
Conversation
Adds a nullable `size` column to file_resource and populates it in bytes wherever a resource's content is actually available: - StorageUtilService.uploadFile() records the size of every locally produced resource (download, manifest, readme, changelog, license, icon, vsixmanifest, signature, sha256) right after upload. - PublishExtensionVersionService.mirrorResource(TempFile) records it for mirrored resources too, since the bytes are extracted from the origin package before being discarded (mirror mode serves file content on the fly rather than storing it). A new Flyway migration (V1_73) adds the column and seeds migration_item rows, most-downloaded extensions first, so a new FileResourceSizeJobRequestHandler backfills the size of every previously published file resource. Consistent with the existing FixMissingFilesMigration, the handler is disabled on mirror instances since most of their resources have no locally stored bytes to measure. jOOQ generated sources were regenerated for the new column.
Contributor
Author
|
Automatically migrating all existing extensions to determine the file sizes for all stored resources will take too long and not make sense, we need to find a better day of doing that. |
…ontent Downloading every previously-published file resource's full content just to measure its size doesn't scale: a large registry (100k+ extensions, reported in review) has orders of magnitude more file resources than that, and every storage backend already tracks an object's size as metadata without needing to transfer its content. Add IStorageService.getFileSize(FileResource), implemented per backend with no content transfer: - LocalStorageService: Files.size(path) (already free) - GoogleCloudStorageService: Storage.get(BlobId) -> Blob.getSize() - AzureBlobStorageService: BlobClient.getProperties().getBlobSize() - AwsStorageService: S3Client.headObject(...) -> HeadObjectResponse.contentLength() Wire it through StorageUtilService.getFileSize (same getStorageServiceForRetrieval dispatch as downloadFile) and MigrationService.getFileSize, and swap FileResourceSizeJobRequestHandler to call it instead of downloading+Files.size on a temp file. Verified against a real LocalStack S3 instance (AwsStorageServiceIntegrationTest.testGetFileSizeWithoutDownloading) and unit tests covering the StorageUtilService dispatch (correct backend, fails fast for an unrecognized storage type). Full server unit suite: 916/916.
Contributor
Author
|
Good catch — pushed a fix (1771ef7). The backfill no longer downloads each file's content to measure it; it now uses a metadata-only lookup per storage backend instead, so no content is ever transferred for the backfill itself:
Added This should make the backfill viable even at 100k+ extensions, since it's now bounded by request count/latency rather than total data transferred. |
MigrationJobRequest<T extends JobRequestHandler<MigrationJobRequest<?>>> was
self-referential: T's bound mentions this very class parametrized with itself
again. Jackson 3's TypeFactory recursively resolves that generic signature
while serializing the `handler` field for JobRunr's queue storage, and never
terminates -- a StackOverflowError wrapped as a DatabindException the moment
any migration item is actually enqueued:
tools.jackson.databind.DatabindException: Infinite recursion (StackOverflowError)
(through reference chain: Job["jobDetails"]->JobDetails["jobParameters"])
at tools.jackson.databind.type.TypeFactory._fromParamType
at tools.jackson.databind.type.TypeFactory._fromAny
... (repeats)
This predates and is unrelated to the FileResourceSizeMigration work; it
affects every migration type sharing this base class. It only surfaced now
because V1_73's backfill seeded the first genuinely new migration_item batch
in a while.
Relax the bound to T extends JobRequestHandler<?>, matching the sibling
HandlerJobRequest class, which was never self-referential and never hit this.
Verified in isolation with plain Jackson 3 that the original bound recurses
and the relaxed one doesn't; MigrationJobRequestTest guards against it via the
same serialization path. Full server unit suite: 917/917.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds persistent tracking of file sizes for FileResource so the registry can support size-based analytics and operational insights. This introduces a new nullable DB column, writes sizes at publish/mirror time, and provides a JobRunr-based backfill that reads size from storage metadata (no content download).
Changes:
- Add
file_resource.size(bytes) and seedmigration_itementries to backfill existing rows. - Record size when publishing (
StorageUtilService.uploadFile) and in mirror mode (PublishExtensionVersionService.mirrorResource). - Add
IStorageService.getFileSize(FileResource)and backend implementations (Local/GCS/Azure/AWS) to support metadata-only backfill, plus new/updated tests.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/main/resources/db/migration/V1_73__File_Resource_Size.sql | Adds size column and seeds backfill migration items ordered by download count. |
| server/src/main/java/org/eclipse/openvsx/entities/FileResource.java | Adds nullable size field with getter/setter and Javadoc. |
| server/src/main/java/org/eclipse/openvsx/storage/IStorageService.java | Introduces getFileSize(FileResource) contract for metadata-only size lookup. |
| server/src/main/java/org/eclipse/openvsx/storage/StorageUtilService.java | Records size at upload time and adds a getFileSize(FileResource) delegator. |
| server/src/main/java/org/eclipse/openvsx/storage/LocalStorageService.java | Implements size lookup via Files.size(...). |
| server/src/main/java/org/eclipse/openvsx/storage/GoogleCloudStorageService.java | Implements size lookup via GCS metadata fetch (no download). |
| server/src/main/java/org/eclipse/openvsx/storage/AzureBlobStorageService.java | Implements size lookup via Azure blob properties (no download). |
| server/src/main/java/org/eclipse/openvsx/storage/AwsStorageService.java | Implements size lookup via S3 HeadObject (no download). |
| server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionService.java | Captures size from extracted mirror temp file prior to persisting. |
| server/src/main/java/org/eclipse/openvsx/migration/MigrationService.java | Registers FileResourceSizeMigration handler and adds retryable getFileSize. |
| server/src/main/java/org/eclipse/openvsx/migration/FileResourceSizeJobRequestHandler.java | New JobRunr handler to backfill missing FileResource.size via metadata lookup. |
| server/src/main/java/org/eclipse/openvsx/migration/MigrationJobRequest.java | Adjusts handler generic bound to avoid Jackson infinite recursion in JobRunr serialization. |
| server/src/test/java/org/eclipse/openvsx/storage/StorageUtilServiceUploadFileTest.java | New unit test verifying upload-time size capture and backend dispatch for size lookups. |
| server/src/test/java/org/eclipse/openvsx/publish/PublishExtensionVersionServiceTest.java | Adds test covering mirror path size capture. |
| server/src/test/java/org/eclipse/openvsx/storage/AwsStorageServiceIntegrationTest.java | Adds integration test verifying size lookup via S3 metadata. |
| server/src/test/java/org/eclipse/openvsx/migration/MigrationJobRequestTest.java | New regression test guarding against Jackson generic-recursion during request serialization. |
| server/src/main/jooq-gen/org/eclipse/openvsx/jooq/tables/FileResource.java | jOOQ regenerated to include SIZE column. |
| server/src/main/jooq-gen/org/eclipse/openvsx/jooq/tables/records/FileResourceRecord.java | jOOQ regenerated to add size accessors/ctor parameter. |
Files not reviewed (2)
- server/src/main/jooq-gen/org/eclipse/openvsx/jooq/tables/FileResource.java: Generated file
- server/src/main/jooq-gen/org/eclipse/openvsx/jooq/tables/records/FileResourceRecord.java: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Remove unused Blob import in GoogleCloudStorageService (var blob never referenced the type by name). - testGetFileSizeWithoutDownloading compared the uploaded string's UTF-16 char count (String.length()) against the stored byte size, which only happened to work for pure-ASCII content. Assert against the actual byte array length used for the upload instead. (MigrationJobRequestTest was also flagged for not declaring/catching a checked exception around JsonMapper.writeValueAsString -- that's a Jackson 2 assumption; Jackson 3's JacksonException is an unchecked RuntimeException, so no throws/try-catch is needed there. Confirmed via javap against jackson-core 3.1.5. Left as-is.) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Sep 5, 2026
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
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.
Summary
Closes #1777.
Adds a nullable
size(bytes) column toFileResource, populates it wherever a resource's content is actually available, and backfills it for existing data.Changes
V1_73__File_Resource_Size.sqladdsfile_resource.size BIGINTand seedsmigration_itemrows (most-downloaded extensions first) for the backfill job. jOOQ generated sources were regenerated for the new column.StorageUtilService.uploadFile()— the single choke point every regular (non-mirror) publish resource passes through (download, manifest, readme, changelog, license, icon, vsixmanifest, signature, sha256) — now records the byte size right after upload.PublishExtensionVersionService.mirrorResource(TempFile)— mirror mode never uploads bytes (it serves content from the origin registry on the fly), but the extractedTempFilestill holds real bytes at that point, so the size is captured there too.FileResourceSizeJobRequestHandler, following the existingmigration_item/JobRunr pattern (same shape as the sha256-checksum and missing-files migrations). Updated per review feedback (thanks @netomi): the handler no longer downloads each file's full content to measure it — at 100k+ extensions that would mean re-transferring an enormous amount of data just to learn a number every storage backend already tracks as metadata. It now does a metadata-only lookup instead:IStorageService.getFileSize(FileResource), implemented per backend with no content transfer:Files.size(path)locally,Storage.get(BlobId)on GCS,BlobClient.getProperties().getBlobSize()on Azure,S3Client.headObject(...)on AWS (a HEAD request).FixMissingFilesMigration), since most of their resources have no locally stored object to look up metadata for in the first place.Out of scope
Exposing file size through a public API/JSON response is left for a follow-up —
ExtensionJson.filesis currently aMap<String,String>(type → URL) and would need a shape change to carry sizes, which felt like a separate API-design decision beyond what the issue asked for (collect, store, backfill).Testing
StorageUtilServiceUploadFileTest— verifiesuploadFilerecords the byte size, and thatgetFileSizedispatches to the right backend / fails fast for an unrecognized storage type.PublishExtensionVersionServiceTest.mirrorResource_recordsTheSizeOfTheExtractedBytes— verifies the mirror path.AwsStorageServiceIntegrationTest.testGetFileSizeWithoutDownloading— against a real LocalStack S3 instance, confirms the size comes back correctly via a HEAD request after upload../gradlew unitTests) passes.V1_73) applies cleanly against a real Postgres instance.🤖 Generated with Claude Code