Conversation
…omposit store ITs
…re size (#2808) - fix test cleanup; data files should be deleted after FileStore is closed
…ven more DB support)
…ven more DB support)
…ven more classes)
…ore.data to oak.spi.blb.data)
…ven more classes)
…ream in order to reduce confusion
|
Note that bad test coverage of course is not caused by this PR. |
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/data/util/NamedThreadFactory.java
Show resolved
Hide resolved
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/data/AbstractBackend.java
Show resolved
Hide resolved
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/data/AbstractBackend.java
Show resolved
Hide resolved
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/data/AsyncTouchCallback.java
Show resolved
Hide resolved
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/data/AsyncUploadCallback.java
Show resolved
Hide resolved
| if (pos > 0) { | ||
| long skipped = is.skip(pos); | ||
| if (skipped < pos) { | ||
| return -1; | ||
| } | ||
| } | ||
|
|
||
| return is.read(buff, off, length); |
There was a problem hiding this comment.
I think we need to use a loop to skip correctly. Something like this:
| if (pos > 0) { | |
| long skipped = is.skip(pos); | |
| if (skipped < pos) { | |
| return -1; | |
| } | |
| } | |
| return is.read(buff, off, length); | |
| long remaining = pos; | |
| while (remaining > 0) { | |
| long skipped = is.skip(remaining); | |
| if (skipped <= 0) { | |
| return -1; | |
| } | |
| remaining -= skipped; | |
| } |
Or use is.skipNBytes(pos) (Java 12+) which throws on short skip, or IOUtils.skipFully() from Commons IO.
And then we should have a test case for this.
There was a problem hiding this comment.
And then we should have a test case for this.
I'll have a look.
There was a problem hiding this comment.
@thomasmueller - the code that instantiates the wrapper has indeed test coverage (Jcr2ToSegmentTest, maybe more), but none of the methods are actually invoked.
Optimally, we would have a test that validates before/after behaviour.
There was a problem hiding this comment.
ok, writing a test that exercises just the skipping method is simple, but it would only prove that skipNBytes works, which we should assume.
The only other thing that actually would be interesting would be a real IT.
Given the fact that this is "almost dead" code anyway, I'd prefer not to do it.
Instead, we should remove the "upgrade" code path (JR->OAK) and just leave the "sidegrade" functionality in. Should I open a separate ticket for that? (cc @mbaedke )
…2811) * OAK-12145 : add compatibility tests for Caffeine migration (PR 2807) Add implementation-independent test coverage for all classes affected by the Guava-to-Caffeine cache migration in OAK-11946. Tests reference only Oak-level types (CacheLIRS, CacheStats, DiffCache, etc.) so the same suite can be cherry-picked to OAK-11946 and run unchanged; any failure there is a migration compatibility gap. Modules covered: - oak-core-spi: AbstractCacheStats, CacheLIRS, EmpiricalWeigher - oak-store-document: DocumentNodeStoreBuilder, NodeDocumentCache, MemoryDiffCache, LocalDiffCache, TieredDiffCache, CachingCommitValueResolver, DocumentNodeStore, PersistentCache - oak-run-commons: DocumentNodeStoreHelper - oak-search: ExtractedTextCache (stats tracking) - oak-search-elastic: ElasticIndexStatistics (cache, refresh, failure) - oak-segment-tar: SegmentCache (loader failure contract) - oak-blob: BlobIdSet (cache miss / persistence semantics) - oak-blob-cloud: S3Backend (expiry, cache enable/disable) - oak-blob-cloud-azure: AzureBlobStoreBackend, AzureBlobStoreBackendV8 (expiry, cache enable/disable) * OAK-12145: Clarify cache compatibility test intent * OAK-12145: Rename S3 compatibility test * OAK-12145: Make cache compatibility tests behavior-based * OAK-12145: Fix SegmentCache compatibility assertion
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/data/util/NamedThreadFactory.java
Show resolved
Hide resolved
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/data/util/package-info.java
Outdated
Show resolved
Hide resolved
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/data/AbstractBackend.java
Show resolved
Hide resolved
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/data/AbstractBackend.java
Show resolved
Hide resolved
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/data/AbstractBackend.java
Show resolved
Hide resolved
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/data/AbstractBackend.java
Show resolved
Hide resolved
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/data/AbstractBackend.java
Show resolved
Hide resolved
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/data/AbstractBackend.java
Show resolved
Hide resolved
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/data/AbstractBackend.java
Show resolved
Hide resolved
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/data/AbstractBackend.java
Show resolved
Hide resolved
|




This change change affects lots of classes, due to refactoring. Inspecting individual commits should be easier. Will explain the different aspects later.
(I believe we should look at IntelliJ/Sonar warnings in a separate step)
--->>> https://issues.apache.org/jira/browse/OAK-8840?focusedCommentId=18068214&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-18068214