Commit 1bfc45b
Port the CLARIN S3 stack to AWS SDK v2 (decision D1)
7.6.7 rewrote S3BitStoreService from com.amazonaws (v1) to software.amazon.awssdk
(v2 CRT client). The original plan rejected that and kept v1; the user reversed it:
"if 7.6.7 is on AWS2, go to AWS2". v1 is past AWS end-of-support anyway.
The fork owns three S3 classes vanilla does not know, so this was not a file takeover:
- S3BitStoreService = vanilla 7.6.7 + exactly three CLARIN items (47 added lines):
getFile(Bitstream), `protected` client so the subclasses can reuse it, and a
configurable pathStyleAccessEnabled (vanilla hardcodes forcePathStyle(true) on an
endpoint override; keeping the property preserves the fork's v1 behaviour and keeps
bitstore.xml valid). getFile is byte-identical to the fork's own v9 implementation
(5e930d1) so the branches do not diverge.
- SyncS3BitStoreService - hand-ported, including the explicit multipart path.
- S3DirectDownloadServiceImpl - presigning moved to S3Presigner, built from the same
credentials/region/endpoint as the bitstore client.
- S3BitStoreServiceIT - taken from 7.6.7 wholesale; the fork's copy was
byte-identical to 7.6.5, so no CLARIN test was lost. LocalStack replaces s3mock,
which only ever spoke v1.
- ClarinS3BitStoreServiceIT - new. The CLARIN-only paths had no test at all.
- bitstore.xml / assetstore.cfg - the merge had dropped vanilla's four new tuning keys
(targetThroughputGbps, minPartSizeBytes, maxConcurrency, s3ChecksumAlgorithm).
Two defects the new tests caught, neither visible by reading the code:
1. v1 returned part ETags unquoted, v2 returns them quoted. The multipart checksum
comparison would have failed on every part, and since that path logs and swallows,
completeMultipartUpload would never run - a silent upload failure.
2. SyncS3BitStoreService.put() never closed the FileOutputStream on the local
assetstore copy. Pre-existing, not a port regression, but it leaks a handle per
stored bitstream and made remove() silently fail to delete the local file.
Evidence (local, this head):
- mvn test-compile -DskipTests: 13/13 modules SUCCESS
- S3BitStoreServiceIT Tests run: 15, Failures: 0, Errors: 0
- ClarinS3BitStoreServiceIT Tests run: 6, Failures: 0, Errors: 0
- S3DirectDownloadServiceTest Tests run: 8, Failures: 0, Errors: 0
- checkstyle 0 violations; `git grep com.amazonaws` finds nothing
s3Store is lazy-init and nothing else instantiates it, so springWiringBindsStoreProperties
loads the bean from bitstore.xml on purpose - it is the only check that the blank
maxConcurrency binds to a null Integer and that s3ChecksumAlgorithm binds to the enum.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>1 parent b284571 commit 1bfc45b
9 files changed
Lines changed: 829 additions & 517 deletions
File tree
- dspace-api
- src
- main/java/org/dspace/storage/bitstore
- test/java/org/dspace/storage/bitstore
- dspace/config
- modules
- spring/api
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
742 | 742 | | |
743 | 743 | | |
744 | 744 | | |
745 | | - | |
746 | | - | |
747 | | - | |
| 745 | + | |
| 746 | + | |
| 747 | + | |
| 748 | + | |
| 749 | + | |
| 750 | + | |
| 751 | + | |
| 752 | + | |
| 753 | + | |
| 754 | + | |
| 755 | + | |
| 756 | + | |
| 757 | + | |
| 758 | + | |
| 759 | + | |
| 760 | + | |
| 761 | + | |
| 762 | + | |
| 763 | + | |
748 | 764 | | |
749 | 765 | | |
750 | 766 | | |
| |||
860 | 876 | | |
861 | 877 | | |
862 | 878 | | |
| 879 | + | |
863 | 880 | | |
864 | | - | |
865 | | - | |
866 | | - | |
867 | | - | |
868 | | - | |
869 | | - | |
870 | | - | |
871 | | - | |
872 | | - | |
873 | | - | |
874 | | - | |
875 | | - | |
876 | | - | |
877 | | - | |
| 881 | + | |
| 882 | + | |
| 883 | + | |
| 884 | + | |
878 | 885 | | |
879 | 886 | | |
880 | 887 | | |
| |||
0 commit comments