Skip to content

refactor: move the download ingestion code to analytics.ingestion - #2131

Open
netomi wants to merge 8 commits into
mainfrom
split/ingestion-move
Open

refactor: move the download ingestion code to analytics.ingestion#2131
netomi wants to merge 8 commits into
mainfrom
split/ingestion-move

Conversation

@netomi

@netomi netomi commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Stack: 2 of 6. Split out of #2027. Base: #2130 (split/webui-library-surface).

A pure relocation with no behaviour change, so that the ingestion rework in the next PR is reviewable as a rework rather than as a rework tangled up with a package move.

storage/log/AwsDownloadCountHandler                       -> analytics/ingestion/aws/
storage/log/{CloudFront,Fastly}LogFileParser, LogFileParser, LogRecord
                                                          -> analytics/ingestion/aws/
storage/log/AzureDownloadCountHandler                     -> analytics/ingestion/azure/
storage/log/{DownloadCountProcessor,DownloadCountService}  -> analytics/ingestion/

How to check it is really a no-op

20 files, +22/−18. Git detects all 12 relocated files as renames with content identical, so git diff -M shows only:

  • package declarations in the moved files
  • imports that used to be same-package and now cross one
  • the DownloadCountService mock declarations in the six tests that name it
  • one pointcut string, below

Filtering the diff to non-package/non-import lines leaves exactly one hunk.

The one thing that could have broken silently

-@Around("execution(* org.eclipse.openvsx.storage.log.*DownloadCountService.isEnabled(..))")
+@Around("execution(* org.eclipse.openvsx.analytics.ingestion.*DownloadCountService.isEnabled(..))")

DownloadCountServiceAspect matches the package as a string. Nothing in the compiler or the type system would have caught this, and the failure mode is quiet: the mirror stops intercepting isEnabled, and download counting behaves differently only in mirror deployments. This is the concrete argument for separating the move from the rework — inside a 114-file feature PR, that line is invisible.

Class names are deliberately not changed here. The DownloadCountProcessedItem -> DownloadIngestion entity rename comes with a table rename, which is a data migration, so it belongs with the rework rather than with a move.

Verification

compileJava/compileTestJava clean; 371 tests passing across the ingestion, storage, registry, admin, VSCode-adapter and Eclipse-service suites (everything that references the moved classes).

Refs #2027, #2025

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The change set is a consistent package relocation with updated references (including the AOP pointcut) and no remaining org.eclipse.openvsx.storage.log references found in the codebase.

Pull request overview

This PR relocates the server-side “download ingestion” implementation from org.eclipse.openvsx.storage.log into the new org.eclipse.openvsx.analytics.ingestion package hierarchy (with AWS/Azure subpackages), aiming to keep subsequent ingestion rework focused and reviewable without mixing in package-move noise.

Changes:

  • Moved ingestion service/processor and AWS/Azure handlers into analytics.ingestion packages (package declarations + import rewrites).
  • Updated all in-repo references (including tests) to import org.eclipse.openvsx.analytics.ingestion.DownloadCountService.
  • Updated the mirror AOP pointcut string to match the new DownloadCountService package.
File summaries
File Description
server/src/test/resources/org/eclipse/openvsx/analytics/ingestion/aws/fastly.log Adds Fastly sample log under the new AWS ingestion resource path for tests.
server/src/test/resources/org/eclipse/openvsx/analytics/ingestion/aws/cloudfront.log Adds CloudFront sample log under the new AWS ingestion resource path for tests.
server/src/test/java/org/eclipse/openvsx/storage/StorageUtilServiceUploadFileTest.java Updates DownloadCountService import to new analytics ingestion package.
server/src/test/java/org/eclipse/openvsx/storage/StorageUtilServiceTest.java Updates DownloadCountService import to new analytics ingestion package.
server/src/test/java/org/eclipse/openvsx/RegistryAPITest.java Updates DownloadCountService import to new analytics ingestion package.
server/src/test/java/org/eclipse/openvsx/eclipse/EclipseServiceTest.java Updates DownloadCountService import to new analytics ingestion package.
server/src/test/java/org/eclipse/openvsx/analytics/ingestion/aws/FastlyLogFileParserTest.java Moves test package to analytics.ingestion.aws to match relocated AWS parser code/resources.
server/src/test/java/org/eclipse/openvsx/analytics/ingestion/aws/CloudFrontLogFileParserTest.java Moves test package to analytics.ingestion.aws to match relocated AWS parser code/resources.
server/src/test/java/org/eclipse/openvsx/admin/AdminAPITest.java Updates DownloadCountService import to new analytics ingestion package.
server/src/test/java/org/eclipse/openvsx/adapter/VSCodeAPITest.java Updates DownloadCountService import to new analytics ingestion package.
server/src/main/java/org/eclipse/openvsx/storage/StorageUtilService.java Updates DownloadCountService import to new analytics ingestion package.
server/src/main/java/org/eclipse/openvsx/mirror/aop/DownloadCountServiceAspect.java Updates AOP pointcut package string to match relocated DownloadCountService.
server/src/main/java/org/eclipse/openvsx/analytics/ingestion/DownloadCountService.java Relocates service to analytics.ingestion; adds cross-package imports for AWS/Azure handlers.
server/src/main/java/org/eclipse/openvsx/analytics/ingestion/DownloadCountProcessor.java Relocates processor to analytics.ingestion (package declaration update).
server/src/main/java/org/eclipse/openvsx/analytics/ingestion/azure/AzureDownloadCountHandler.java Relocates Azure handler into analytics.ingestion.azure and adjusts imports accordingly.
server/src/main/java/org/eclipse/openvsx/analytics/ingestion/aws/LogRecord.java Relocates AWS log record model into analytics.ingestion.aws.
server/src/main/java/org/eclipse/openvsx/analytics/ingestion/aws/LogFileParser.java Relocates AWS log parser interface into analytics.ingestion.aws.
server/src/main/java/org/eclipse/openvsx/analytics/ingestion/aws/FastlyLogFileParser.java Relocates Fastly parser into analytics.ingestion.aws.
server/src/main/java/org/eclipse/openvsx/analytics/ingestion/aws/CloudFrontLogFileParser.java Relocates CloudFront parser into analytics.ingestion.aws.
server/src/main/java/org/eclipse/openvsx/analytics/ingestion/aws/AwsDownloadCountHandler.java Relocates AWS handler into analytics.ingestion.aws and adjusts imports accordingly.
Review details
  • Files reviewed: 18/20 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

gnugomez and others added 8 commits September 3, 2026 16:46
The dashboard spec is the first test to mount a page that reaches @mui/x-charts
(the rate limiting usage stats chart), x-data-grid and x-date-pickers. Their ESM
builds import directory paths Node's resolver rejects, so vite has to bundle them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
adminPages is consumer-provided API input, and the shadowing check took the first
segment of the path as given. A leading slash left it inspecting an empty
segment, so '/customers' passed the check and sat in the nav beside the built-in
page it names, and createRoute joined it into '/admin-dashboard//customers'.

Paths are now trimmed of leading and trailing slashes before anything derives a
route or a nav link from them, and a path left empty by that is dropped.

Also corrects two doc comments that said the opposite of what the code does:
compactControl carries nested selectors for the select's inner slots, so it is
sx-only rather than usable in a plain style prop; and useExtendNavbarBlur has no
ownership stack, so "last writer wins" only holds until any writer unmounts and
resets the fan under a second one still mounted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pure relocation, no behaviour change, ahead of the ingestion rework. The files
move from org.eclipse.openvsx.storage.log to the package layout the rework
targets:

  storage/log/AwsDownloadCountHandler    -> analytics/ingestion/aws/
  storage/log/{CloudFront,Fastly}LogFileParser, LogFileParser, LogRecord
                                         -> analytics/ingestion/aws/
  storage/log/AzureDownloadCountHandler  -> analytics/ingestion/azure/
  storage/log/{DownloadCountProcessor,DownloadCountService}
                                         -> analytics/ingestion/

Class names and file contents are untouched, so every file is a git rename and
the only edits are `package` declarations, the imports that were previously
same-package, and the mock declarations in the tests that name the service.

The one change a compiler could not have caught is in DownloadCountServiceAspect:
its @around pointcut matches the package as a string, so the move would have
silently stopped the mirror from intercepting isEnabled. That is the reason for
doing this as its own step.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@netomi
netomi force-pushed the split/ingestion-move branch from 3af6963 to 39f5d25 Compare September 3, 2026 14:46
Base automatically changed from split/webui-library-surface to main September 3, 2026 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants