Skip to content

Fix trailing slash, response leak, NPE, and minor issues in module system#6930

Merged
pditommaso merged 3 commits intofix/module-system-code-issuesfrom
fix/module-system-code-issues-2
Mar 17, 2026
Merged

Fix trailing slash, response leak, NPE, and minor issues in module system#6930
pditommaso merged 3 commits intofix/module-system-code-issuesfrom
fix/module-system-code-issues-2

Conversation

@pditommaso
Copy link
Copy Markdown
Member

Summary

Stacked on #6929. Fixes 8 additional issues found during code review:

Important fixes:

  • Trailing slash in registry URLs causes 404s -- RegistryConfig now strips trailing slashes from configured URLs, preventing double-slash paths like https://registry.../api//v1/modules/
  • HTTP response body InputStream not closed -- ModuleRegistryClient.downloadModule() now wraps response.body() in try-with-resources after Files.copy() to avoid leaking connections
  • NPE when release.metadata is null -- CmdModuleInfo logged a warning but continued to access metadata.description, causing NPE. Now throws AbortOperationException instead
  • Inconsistent ModuleInfo.load() return types -- load(Path) returned [:] while load(Path, String) returned null for missing files. Now both return null consistently

Minor fixes:

  • Duplicate isRemoteModule regex -- Extracted pattern to ModuleResolver.REMOTE_MODULE_PATTERN constant; added cross-reference comment in ModuleReference to keep them in sync
  • removeModule returns null instead of false -- Bare return in a boolean method now explicitly returns false
  • Typo in error message -- "No module diretory" fixed to "No module directory" in CmdModulePublish
  • Unclosed Files.walk() stream -- ModuleStorageTest.createTestPackage() now closes the stream via withCloseable{}

Test plan

  • ModuleInfoTest passes (updated to expect null instead of [:])
  • ModuleStorageTest passes
  • CmdModuleInfoTest passes
  • CmdModulePublishTest passes
  • ModuleReferenceTest passes
  • Full compilation succeeds

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso pditommaso requested a review from a team as a code owner March 17, 2026 09:00
@pditommaso pditommaso requested a review from jorgee March 17, 2026 09:00
* Use shared RegistryClient from npr-client for module commands

Replace ModuleRegistryClient with RegistryClient from the new
io.seqera:npr-client library. This moves the registry HTTP client
and checksum algorithm to a shared library used by both Nextflow
and the plugin-registry server.

Changes:
- ModuleChecksum now delegates to io.seqera.npr.client.ModuleChecksum
- ModuleRegistryClient removed, replaced by RegistryClientFactory
  that creates RegistryClient instances from RegistryConfig
- All CLI commands and ModuleResolver use RegistryClient directly
- Tests updated to use RegistryClient type for mocks/stubs
- Exception contract changed from AbortOperationException to
  RegistryException for registry operations

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>

* fix import sort

Signed-off-by: jorgee <jorge.ejarque@seqera.io>

---------

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: jorgee <jorge.ejarque@seqera.io>
@pditommaso pditommaso merged commit 946454b into fix/module-system-code-issues Mar 17, 2026
4 checks passed
@pditommaso pditommaso deleted the fix/module-system-code-issues-2 branch March 17, 2026 13:11
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.

2 participants