Feature/storage provider architecture - #107
Merged
Merged
Conversation
This document outlines the architecture for abstracting storage providers in the data download server to support multiple backends (NFS, S3, openBIS DSS). Key features: - Clean StorageProvider interface with multiple access patterns - Provider registry for dataset-to-provider mapping - Gradual migration strategy with parallel implementation - Backward compatible API endpoints - Simplified architecture removing producer-consumer complexity The architecture enables: - Direct file I/O for NFS (better performance) - Pre-signed URLs for S3 (client direct access) - Adapter pattern for existing openBIS code (low risk) - Configurable per-user limits and retry strategies - Future-proof design for additional storage backends Implementation plan includes 6 phases over 10 weeks with detailed deliverables, testing strategy, and risk mitigation.
Split optional capabilities into FilePathProvider and PresignedUrlProvider role interfaces; add exception taxonomy and explicit byte-range contract.
Split range-capable getFile out of StorageProvider into a ByteRangeProvider role interface so whole-file-only providers keep a lean core.
Size is already available via FileInfo from getFileMetadata, so the redundant method is removed from the interface.
Do not mandate CRC32 in the interface; expose integrity checksums as an algorithm+value pair chosen by the storage provider.
The providers map key is now the provider id the registry resolves to; the type property decides which other properties are required.
Uses openBIS for metadata and physical path resolution but streams file content directly from the mounted disc via NIO instead of through the DSS HTTP API. Documented in diagram, config, phase 3, glossary.
* feat: add storage provider abstraction with openbis adapter Add a new storage-provider module exposing the lean StorageProvider interface, capability interfaces, FileInfo/DataFile/ByteRange types and the StorageProviderException taxonomy. Implement OpenBisStorageProvider adapting the legacy MeasurementDataProvider (openBIS) to this contract with byte-range support. * docs: declare thrown exceptions on storage interfaces Add throws clauses to the interface methods so callers can see the failure modes of each operation. Exceptions remain unchecked, so the declarations are informational and do not force handling. * perf: cache openbis file listing in adapter Multiple file and byte-range requests on the same dataset each triggered an openBIS listFiles call. Cache the path-sorted listing per dataset for a short TTL (default 30s) so requests issued close together share one listing. Datasets change very seldomly, so the cache stays valid. * refactor: drop byte-range support from openbis adapter OpenBisStorageProvider no longer implements ByteRangeProvider; it serves whole files only. Range handling is left to providers that support it natively (NFS, S3). * feat: model all RFC 9110 byte-range forms Replace the start/end record with a sealed ByteRange covering int-range (both bounds), open-ended int-range and suffix-range. Add resolve(fileSize) to compute concrete bounds and a ByteRangeParser for Range header values. Byte offsets are inclusive per RFC 9110 section 14.1.2. * refactor: rename ByteRange.Resolved to ResolvedRange Clarify that resolve() yields a concrete range, not an opaque result. --------- Co-authored-by: KochTobi-Agent <kochtobi-agent@users.noreply.github.com>
Add ProviderRegistry abstraction in storage-provider (interface, ProviderDefinition, ProviderFactory, DatasetProviderResolver, ConfigurableProviderRegistry) and wire it in rest-api via providers.* @ConfigurationProperties, an openbis ProviderFactory and a config-backed resolver. The registry is not yet consumed by endpoints. Co-authored-by: KochTobi-Agent <kochtobi-agent@users.noreply.github.com>
Add MeasurementFileControllerV2 using StorageProvider abstraction: - StorageFileIndex: file listing cache backed by ProviderRegistry - MeasurementFileControllerV2: uses StorageProvider with ByteRangeProvider capability detection for resumable downloads - Feature flag (download.controller-version=v1|v2, default v1) for gradual rollout without downtime - Exception mapping: DatasetNotFoundException -> 404, InvalidByteRangeException -> 416, StorageFileNotFoundException -> 404 - Revert SecurityConfig redirectToHttps debugging change - Enable provider registry configuration in application.properties Tests: - StorageFileIndexTest: index resolution, caching, eviction - MeasurementFileControllerV2Test: manifest, download, range handling, error mapping (10 tests) - ControllerVersionSwitchTest: V1 default, V1 explicit, V2 activation All 70 tests pass.
Implement OpenBisNfsStorageProvider that combines openBIS metadata with direct NFS file streaming via NIO: - Uses MeasurementDataProvider for file metadata (list, order, checksums) - Resolves physical paths by combining mount-path with openBIS file paths - Streams files using Java NIO FileChannel for efficient I/O - Implements StorageProvider + ByteRangeProvider + FilePathProvider - Supports resumable downloads via native byte-range requests - Caches file listings to limit openBIS traffic Configuration: - Provider type: 'openbis-nfs' - Required property: 'mount-path' (root directory of mounted openBIS data) - Wired into ProviderRegistryConfig with validation Testing: - OpenBisNfsStorageProviderTest: 12 unit tests covering file listing, streaming, byte ranges, error handling, caching, validation - ProviderRegistryConfigOpenBisNfsTest: 3 integration tests verifying provider type wiring and configuration validation All 73 tests pass (70 existing + 3 new). This enables high-performance NFS-based downloads while maintaining openBIS metadata richness (checksums, timestamps, file ordering).
- Add openbis-nfs provider configuration with environment variables: - OPENBIS_NFS_MOUNT_PATH: local directory for NFS test files - DEFAULT_PROVIDER_ID: switch between openbis-1 and openbis-nfs-1 - Add setup-nfs-test.sh script to create test directory structure matching openBIS file paths for local NFS provider testing Usage: ./setup-nfs-test.sh <measurement-id> export OPENBIS_NFS_MOUNT_PATH=/tmp/openbis-nfs-test export DEFAULT_PROVIDER_ID=openbis-nfs-1 cd rest-api && mvn spring-boot:run -DskipTests
Added INFO-level logging to both OpenBisStorageProvider (HTTP) and OpenBisNfsStorageProvider (NFS) to log when listFiles is called. This helps verify which provider is active during testing: - [HTTP Provider] listFiles called for dataset: <id> - [NFS Provider] listFiles called for dataset: <id>
…on from openBIS - OpenBisNfsStorageProvider now fetches physical storage location from openBIS using DataSet.getPhysicalData().getLocation() instead of assuming mount-path directly maps to file paths - Added loadDataSetsForMeasurement() public method to OpenBisConnector to expose DataSet objects with physical data information - Updated ProviderRegistryConfig to inject OpenBisConnector for openbis-nfs provider type, made it optional to support test configurations - Simplified OpenBisNfsStorageProviderTest to focus on validation tests since full integration tests require real openBIS connection - Updated ProviderRegistryConfig tests to work with optional OpenBisConnector All 71 tests pass. The NFS provider now correctly: 1. Fetches DataSet metadata from openBIS with physical data 2. Extracts the physical storage location from the DataSet 3. Maps the location to the local NFS mount path 4. Streams files using Java NIO for high performance
- Updated OpenBisNfsStorageProvider to correctly resolve physical paths by combining: mountPath + physicalLocation + filePath - Physical location from openBIS (e.g., D1B57258-.../c0/0d/c3/...) is now properly used as the base directory for file resolution - Removed debug System.out.println statements - Updated setup-nfs-test.sh to support sharded directory structure - Script now requires physical location as second parameter - Creates test files under the sharded structure - Provides instructions to get physical location from server logs This ensures the NFS provider correctly mimics openBIS's sharded storage layout where files are stored under a physical location path that includes UUID-based sharding directories.
…erface - Added getPhysicalLocation() method to MeasurementDataProvider interface with default implementation returning Optional.empty() - Implemented getPhysicalLocation() in OpenBisConnector to expose the physical storage location from openBIS DataSet - Changed OpenBisNfsStorageProvider to depend on MeasurementDataProvider interface instead of concrete OpenBisConnector class - Updated ProviderRegistryConfig to use the interface-based approach - Removed unused imports and simplified the provider factory This improves the abstraction by: 1. Using the interface instead of concrete implementation 2. Making the provider more testable and flexible 3. Following the dependency inversion principle All 71 tests pass.
- Reverted MeasurementDataProvider interface to original state (removed getPhysicalLocation) - OpenBisNfsStorageProvider now depends on OpenBisConnector directly - Uses FilePathProvider capability interface correctly - getFilePath() returns the correct path: mount-path + physical-location + relative-path - Updated ProviderRegistryConfig to inject OpenBisConnector for NFS provider - Removed unused PhysicalLocationProvider interface This properly follows the Interface Segregation Principle: - MeasurementDataProvider remains clean and focused on file listing/loading - OpenBisNfsStorageProvider uses FilePathProvider capability as designed - Physical location details are internal to the NFS provider implementation All 71 tests pass.
- Updated ProviderRegistryConfig to create provider-specific OpenBisConnector
instances with their own configuration (credentials, server URLs, etc.)
- Each openbis-nfs provider can now have its own complete openBIS configuration
matching the architecture document specification
- Added helper methods to extract required and optional properties from
provider definitions
- Updated application.properties with example configuration showing all
required properties for openbis-nfs provider
- Updated test configurations to provide SessionFactory beans
Configuration example:
providers.providers.openbis-nfs-1.type=openbis-nfs
providers.providers.openbis-nfs-1.properties.user.name=${OPENBIS_USER_NAME}
providers.providers.openbis-nfs-1.properties.user.password=${OPENBIS_USER_PASSWORD}
providers.providers.openbis-nfs-1.properties.server.application-url=${OPENBIS_APPLICATION_URL}
providers.providers.openbis-nfs-1.properties.server.datastore-urls=${OPENBIS_DATASTORE_URLS}
providers.providers.openbis-nfs-1.properties.filename.ignored-prefix=${OPENBIS_FILE_IGNORED_PREFIX:original}
providers.providers.openbis-nfs-1.properties.mount-path=/mnt/openbis-data
This allows multiple openBIS instances with different credentials and
configurations to be used by different providers.
All 71 tests pass.
Spring Boot's @ConfigurationProperties binds nested properties like 'user.name' as nested Maps, not as flat keys. Added getNestedProperty() helper method to navigate nested map structures using dot notation. This allows provider configuration like: providers.providers.openbis-nfs-1.properties.user.name=${OPENBIS_USER_NAME} providers.providers.openbis-nfs-1.properties.server.application-url=${OPENBIS_APPLICATION_URL} All 71 tests pass.
Replaced generic Map<String, Object> properties with typed configuration classes (UserConfig, ServerConfig, FilenameConfig) in ProviderProperties. This provides: - Type safety for configuration values - Better IDE support and autocomplete - Clearer configuration structure matching the architecture document - Validation through Spring Boot's configuration properties binding Configuration format is now: providers.providers.openbis-nfs-1.user.name=... providers.providers.openbis-nfs-1.server.application-url=... providers.providers.openbis-nfs-1.mount-path=... Instead of the previous nested properties map approach.
Changed ProviderProperties to use 'instances' field instead of 'providers' to avoid the redundant 'providers.providers' configuration path. Configuration is now: providers.instances.openbis-nfs-1.type=openbis-nfs providers.instances.openbis-nfs-1.user.name=... providers.instances.openbis-nfs-1.server.application-url=... providers.default-provider=openbis-nfs-1 Instead of the previous: providers.providers.openbis-nfs-1.type=openbis-nfs ... This provides a cleaner, more intuitive configuration structure.
The NFS provider was using file sizes from openBIS metadata, which could be incorrect or zero. This caused byte range requests to fail with 416 errors when the metadata size didn't match the actual file size. Now the provider reads the actual file size from the filesystem using Files.size() for: - listFiles() - returns correct sizes in file listings - getFile() - uses actual size for range resolution - getFileMetadata() - returns correct size in metadata This ensures resumable downloads work correctly with proper byte range support. All 71 tests pass.
- Added downloadMeasurementAsZip() method to MeasurementFileControllerV2
- Endpoint: GET /measurements/{measurementId} returns ZIP archive
- Uses StorageProvider abstraction for provider-agnostic file access
- Supports all storage providers (NFS, HTTP, etc.)
- Added @ConditionalOnProperty to old MeasurementZipDownloadController
to disable it when V2 is active (v1 controller only)
- ZIP includes all files with proper directory structure
- No resume support for ZIP (dynamic generation), but individual
file downloads support resumable byte-range requests
Tested successfully with NFS provider:
- ZIP download: 7 files, 1.3KB archive
- Individual file download with byte ranges: working
- File content integrity: verified
…name
- Renamed 'filename.ignored-prefix' to 'filename.wrapper-directory' for clarity
The wrapper directory is the intermediate directory created by openBIS DSS
between the sharded storage path and the actual dataset files.
- Updated OpenBisNfsStorageProvider to:
* Accept wrapperDirectory as constructor parameter
* Insert wrapper directory in physical path resolution:
mountPath + physicalLocation + wrapperDirectory + relativePath
* Strip wrapper directory from user-facing file paths
(e.g., 'original/Fastq1/file.gz' -> 'Fastq1/file.gz')
- Fixed ZIP download filename to use measurement ID only
(e.g., 'NGSQ27O50001A0-1481421688841090.zip' instead of timestamp)
- Updated tests to match new constructor signature
- All 71 tests pass
The actual DSS directory structure is: <mount-path>/<sharded-path>/original/<task-id-uuid4>/<dataset-content> Where task-id is a UUID4 assigned during registration. Changes: - Added discoverTaskId() to scan wrapper directory for UUID4 subdirectory - Updated resolvePhysicalPath() to include task-id in path resolution - Added stripWrapperAndTaskId() helper to remove both wrapper and task-id from user-facing paths - Updated all path stripping logic to use the new helper - End users now see clean paths without internal DSS structure details Example: Physical: /mnt/nfs/D1B57258-.../c0/0d/c3/.../original/550e8400-e29b-.../Fastq1/file.gz User sees: Fastq1/file.gz
Add a documented external application.properties template plus README instructions so production configuration is editable in one file instead of relying on exported environment variables.
sven1103-agent
force-pushed
the
feature/storage-provider-architecture
branch
from
September 1, 2026 06:04
b7000cc to
6c6df39
Compare
Add a 'Building for production' section covering prerequisites, the multi-module structure, full and scoped build commands, clean rebuilds, and Nexus deployment.
The child POMs had drifted from the parent version (root 1.3.0 while
several modules still pinned 1.0.10, and parent <version> references
were inconsistent). Consolidate the whole project onto one version
defined once in the root pom:
- root declares <version>${revision}</version> with <revision>1.3.0</revision>
- child modules no longer declare their own <version>; they inherit the
parent and reference it via ${revision}
- inter-module dependencies use ${project.version} so they stay in sync
Update the release workflow to bump the revision property (versions:set
would have overwritten the ${revision} expression); set-property keeps
the central property as the single source of truth. Document the scheme
in the README.
The external application.properties was not being picked up in production, so the bundled defaults (e.g. openbis-nfs mount-path /tmp/openbis-nfs-test) took effect and the server failed to start. Spring Boot loads the file relative to the process working directory, not the JAR's location. Document the guaranteed launch methods (--spring.config.additional-location with an absolute path, or systemd WorkingDirectory) and leave the mount-path unset with a warning so a placeholder default cannot silently be used.
logNearFullQueue ran once per consumed buffer chunk with no time throttling, so a download whose queue stayed near-full for the whole transfer (slow client) produced a WARN for every ~1MB buffer - flooding the logs. Throttle the warning to at most one per progressLogIntervalMs, mirroring the existing progress-log throttling. The throttle state is tracked per download, so concurrent transfers do not suppress each other. Fixes both the legacy MeasurementFileController and the v2 controller.
- Delete V1 controller, zip controller, MeasurementFileIndex, ByteRange, MeasurementDataReaderFactory and their tests - Rename MeasurementFileControllerV2 -> MeasurementFileController - Remove @ConditionalOnProperty and download.controller-version config - Remove measurementDataReaderFactory bean from AppConfig
|
sven1103
marked this pull request as ready for review
September 1, 2026 09:40
sven1103
approved these changes
Sep 1, 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
4 participants
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.
No description provided.