From bb854e1c58ae9a10fbae03e2ca20f188c110ba8f Mon Sep 17 00:00:00 2001 From: Sergey Rymsha Date: Mon, 23 Dec 2024 15:46:00 +0100 Subject: [PATCH] Blobstore can throw FileAlreadyExistsException #10447 (#10831) --- .../blobstore/file/FileBlobStore.java | 15 +- .../blobstore/file/FileBlobStoreTest.java | 13 -- .../blobstore/cache/CacheBlobRecord.java | 58 ------- .../blobstore/cache/CachedBlobStore.java | 92 +++++++++-- .../blobstore/cache/CachedBlobStoreTest.java | 93 ++++++----- .../internal/blobstore/MemoryBlobRecord.java | 5 + .../internal/blobstore/MemoryBlobStore.java | 5 +- .../htmlarea/HtmlAreaDumpUpgrader.java | 2 +- .../impl/node/dao/NodeVersionServiceImpl.java | 149 +++++++++--------- .../impl/node/json/ImmutableVersionData.java | 9 +- .../node/json/NodeVersionJsonSerializer.java | 19 +-- .../node/dao/NodeVersionServiceImplTest.java | 8 +- .../blob/AbstractBlobVacuumTaskTest.java | 28 +--- 13 files changed, 241 insertions(+), 255 deletions(-) delete mode 100644 modules/core/core-blobstore/src/main/java/com/enonic/xp/internal/blobstore/cache/CacheBlobRecord.java diff --git a/modules/blobstore/blobstore-file/src/main/java/com/enonic/xp/internal/blobstore/file/FileBlobStore.java b/modules/blobstore/blobstore-file/src/main/java/com/enonic/xp/internal/blobstore/file/FileBlobStore.java index ebdc6b450eb..3860e4d26df 100644 --- a/modules/blobstore/blobstore-file/src/main/java/com/enonic/xp/internal/blobstore/file/FileBlobStore.java +++ b/modules/blobstore/blobstore-file/src/main/java/com/enonic/xp/internal/blobstore/file/FileBlobStore.java @@ -2,10 +2,10 @@ import java.io.IOException; import java.nio.file.DirectoryNotEmptyException; +import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; -import java.nio.file.attribute.FileTime; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -172,11 +172,14 @@ private BlobRecord addRecord( final Segment segment, final BlobKey key, final By if ( !Files.exists( file ) ) { Files.createDirectories( file.getParent() ); - in.copyTo( MoreFiles.asByteSink( file ) ); - } - else - { - Files.setLastModifiedTime( file, FileTime.fromMillis( System.currentTimeMillis() ) ); + try (var inStream = in.openStream()) + { + Files.copy( inStream, file ); + } + catch ( FileAlreadyExistsException e ) + { + LOG.debug( "File already exists [{}]", file, e ); + } } return new FileBlobRecord( key, file ); diff --git a/modules/blobstore/blobstore-file/src/test/java/com/enonic/xp/internal/blobstore/file/FileBlobStoreTest.java b/modules/blobstore/blobstore-file/src/test/java/com/enonic/xp/internal/blobstore/file/FileBlobStoreTest.java index 37231c84c6a..59ec0f894b1 100644 --- a/modules/blobstore/blobstore-file/src/test/java/com/enonic/xp/internal/blobstore/file/FileBlobStoreTest.java +++ b/modules/blobstore/blobstore-file/src/test/java/com/enonic/xp/internal/blobstore/file/FileBlobStoreTest.java @@ -19,7 +19,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; public class FileBlobStoreTest { @@ -120,18 +119,6 @@ public void deleteSegment() assertEquals( segment, blobStore.listSegments().findFirst().get() ); } - @Test - public void lastModifiedUpdated() - throws Exception - { - final BlobRecord rec1 = this.blobStore.addRecord( this.segment, ByteSource.wrap( "hello".getBytes() ) ); - final long beforeUpdate = rec1.lastModified(); - Thread.sleep( 1000 ); // ensure a second difference - final BlobRecord rec2 = this.blobStore.addRecord( this.segment, ByteSource.wrap( "hello".getBytes() ) ); - - assertTrue( beforeUpdate < rec2.lastModified() ); - } - private BlobRecord createRecord( final String str ) { return createRecord( segment, str ); diff --git a/modules/core/core-blobstore/src/main/java/com/enonic/xp/internal/blobstore/cache/CacheBlobRecord.java b/modules/core/core-blobstore/src/main/java/com/enonic/xp/internal/blobstore/cache/CacheBlobRecord.java deleted file mode 100644 index f40f83e4fc9..00000000000 --- a/modules/core/core-blobstore/src/main/java/com/enonic/xp/internal/blobstore/cache/CacheBlobRecord.java +++ /dev/null @@ -1,58 +0,0 @@ -package com.enonic.xp.internal.blobstore.cache; - -import java.io.IOException; -import java.io.UncheckedIOException; - -import com.google.common.io.ByteSource; - -import com.enonic.xp.blob.BlobKey; -import com.enonic.xp.blob.BlobRecord; - -public class CacheBlobRecord - implements BlobRecord -{ - private final BlobKey blobKey; - - private final long lastModified; - - private final ByteSource content; - - public CacheBlobRecord( final BlobRecord blobRecord ) - throws IOException - { - this.blobKey = blobRecord.getKey(); - this.content = ByteSource.wrap( blobRecord.getBytes().read() ); - this.lastModified = blobRecord.lastModified(); - } - - @Override - public long lastModified() - { - return this.lastModified; - } - - @Override - public BlobKey getKey() - { - return this.blobKey; - } - - @Override - public long getLength() - { - try - { - return content.size(); - } - catch ( IOException e ) - { - throw new UncheckedIOException( e ); - } - } - - @Override - public ByteSource getBytes() - { - return content; - } -} diff --git a/modules/core/core-blobstore/src/main/java/com/enonic/xp/internal/blobstore/cache/CachedBlobStore.java b/modules/core/core-blobstore/src/main/java/com/enonic/xp/internal/blobstore/cache/CachedBlobStore.java index b3c80474f7c..7ba847a7f57 100644 --- a/modules/core/core-blobstore/src/main/java/com/enonic/xp/internal/blobstore/cache/CachedBlobStore.java +++ b/modules/core/core-blobstore/src/main/java/com/enonic/xp/internal/blobstore/cache/CachedBlobStore.java @@ -1,6 +1,8 @@ package com.enonic.xp.internal.blobstore.cache; import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.Objects; import java.util.stream.Stream; import org.slf4j.Logger; @@ -76,7 +78,6 @@ public void removeRecord( final Segment segment, final BlobKey key ) throws BlobStoreException { this.store.removeRecord( segment, key ); - this.cache.invalidate( key ); } @Override @@ -87,21 +88,32 @@ public void invalidate( final Segment segment, final BlobKey key ) private BlobRecord addToCache( final BlobRecord record ) { - if ( record.getLength() <= this.sizeThreshold ) + // Quick check to avoid caching large blobs + if ( record.getLength() > this.sizeThreshold ) { - try - { - final CacheBlobRecord cacheBlobRecord = new CacheBlobRecord( record ); - this.cache.put( record.getKey(), cacheBlobRecord ); - return cacheBlobRecord; - } - catch ( IOException e ) - { - LOG.error( "Could not create cache blob-record", e ); - } + return record; + } + + // Don't do heavy lifting if the blob is already in the cache + final BlobKey key = record.getKey(); + final CacheBlobRecord present = this.cache.getIfPresent( key ); + if ( present != null ) + { + return present; } - return record; + try + { + // We don't want return blobs that are not in cache - as we want them to garbage collected ASAP, + // so we dont use get() with loader here + this.cache.put( key, new CacheBlobRecord( record ) ); + } + catch ( IOException e ) + { + LOG.warn( "Could not cache blob-record with key {}", key, e ); + } + // If we could not load the blob into cache, we return the original record + return Objects.requireNonNullElse( this.cache.getIfPresent( key ), record ); } @Override @@ -171,4 +183,58 @@ public int weigh( final BlobKey key, final BlobRecord record ) return (int) Math.min( record.getLength(), Integer.MAX_VALUE ); } } + + private static final class CacheBlobRecord + implements BlobRecord + { + final BlobKey blobKey; + + final long lastModified; + + final ByteSource content; + + CacheBlobRecord( final BlobRecord blobRecord ) + throws IOException + { + this.content = ByteSource.wrap( blobRecord.getBytes().read() ); + this.blobKey = BlobKey.from( this.content ); + if ( !blobRecord.getKey().equals( this.blobKey ) ) + { + throw new IOException( String.format( "Cache BlobKey must be the same as the key of the BlobRecord: %s != %s", this.blobKey, + blobRecord.getKey() ) ); + } + this.lastModified = blobRecord.lastModified(); + } + + @Override + public BlobKey getKey() + { + return this.blobKey; + } + + @Override + public long getLength() + { + try + { + return content.size(); + } + catch ( IOException e ) + { + throw new UncheckedIOException( e ); + } + } + + @Override + public ByteSource getBytes() + { + return this.content; + } + + @Override + public long lastModified() + { + return this.lastModified; + } + } } diff --git a/modules/core/core-blobstore/src/test/java/com/enonic/xp/internal/blobstore/cache/CachedBlobStoreTest.java b/modules/core/core-blobstore/src/test/java/com/enonic/xp/internal/blobstore/cache/CachedBlobStoreTest.java index a5b01279d08..5914631f04d 100644 --- a/modules/core/core-blobstore/src/test/java/com/enonic/xp/internal/blobstore/cache/CachedBlobStoreTest.java +++ b/modules/core/core-blobstore/src/test/java/com/enonic/xp/internal/blobstore/cache/CachedBlobStoreTest.java @@ -1,6 +1,5 @@ package com.enonic.xp.internal.blobstore.cache; -import java.time.Instant; import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; @@ -13,9 +12,8 @@ import com.enonic.xp.blob.BlobRecord; import com.enonic.xp.blob.BlobStore; import com.enonic.xp.blob.Segment; -import com.enonic.xp.internal.blobstore.MemoryBlobStore; +import com.enonic.xp.internal.blobstore.MemoryBlobRecord; -import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -39,25 +37,20 @@ public void setup() build(); } - private BlobRecord newRecord( final String key, final long size ) + private BlobRecord newRecord() { - return newRecord( key, size, System.currentTimeMillis() ); + return new MemoryBlobRecord( ByteSource.wrap( new byte[10] ) ); } - private BlobRecord newRecord( final String key, final long size, final long lastModified ) + private BlobRecord newLargeRecord() { - final BlobRecord record = Mockito.mock( BlobRecord.class ); - Mockito.when( record.getKey() ).thenReturn( BlobKey.from( key ) ); - Mockito.when( record.getLength() ).thenReturn( size ); - Mockito.when( record.getBytes() ).thenReturn( ByteSource.wrap( "these are my bytes".getBytes() ) ); - Mockito.when( record.lastModified() ).thenReturn( lastModified ); - return record; + return new MemoryBlobRecord( ByteSource.wrap( new byte[20] ) ); } @Test public void getSmallRecord() { - final BlobRecord record = newRecord( "0123", 10L ); + final BlobRecord record = newRecord(); assertNull( this.cachedBlobStore.getRecord( segment, record.getKey() ) ); Mockito.when( this.blobStore.getRecord( segment, record.getKey() ) ).thenReturn( record ); @@ -74,7 +67,7 @@ public void getSmallRecord() @Test public void addSmallRecord() { - final BlobRecord record = newRecord( "0123", 10L ); + final BlobRecord record = newRecord(); final ByteSource byteSource = ByteSource.wrap( "0123".getBytes() ); Mockito.when( this.blobStore.getRecord( segment, record.getKey() ) ).thenReturn( record ); Mockito.when( this.blobStore.addRecord( segment, byteSource ) ).thenReturn( record ); @@ -90,9 +83,10 @@ public void addSmallRecord() @Test public void addLargeRecord() + throws Exception { - final BlobRecord record = newRecord( "0123", 20L ); - final ByteSource byteSource = ByteSource.wrap( "0123".getBytes() ); + final BlobRecord record = newLargeRecord(); + final ByteSource byteSource = ByteSource.wrap( record.getBytes().read() ); Mockito.when( this.blobStore.getRecord( segment, record.getKey() ) ).thenReturn( record ); Mockito.when( this.blobStore.addRecord( segment, byteSource ) ).thenReturn( record ); @@ -109,7 +103,7 @@ public void addLargeRecord() public void removeRecord() throws Exception { - final BlobRecord record = newRecord( "0123", 10L ); + final BlobRecord record = newRecord(); final ByteSource byteSource = ByteSource.wrap( "0123".getBytes() ); Mockito.when( this.blobStore.addRecord( segment, byteSource ) ).thenReturn( record ); @@ -125,7 +119,7 @@ public void removeRecord() public void invalidate() throws Exception { - final BlobRecord record = newRecord( "0123", 10L ); + final BlobRecord record = newRecord(); Mockito.when( this.blobStore.getRecord( segment, record.getKey() ) ).thenReturn( record ); this.cachedBlobStore.getRecord( this.segment, record.getKey() ); @@ -143,7 +137,7 @@ public void invalidate() public void lastModified() throws Exception { - final BlobRecord record = newRecord( "0123", 10L, Instant.now().toEpochMilli() ); + final BlobRecord record = newRecord(); Mockito.when( this.blobStore.getRecord( segment, record.getKey() ) ).thenReturn( record ); final BlobRecord firstRetrieval = this.cachedBlobStore.getRecord( this.segment, record.getKey() ); @@ -155,33 +149,6 @@ public void lastModified() assertEquals( secondRetrieval.lastModified(), record.lastModified() ); } - @Test - public void lastModified_updated() - throws Exception - { - final MemoryBlobStore memoryBlobStore = new MemoryBlobStore(); - final ByteSource source = ByteSource.wrap( "abc".getBytes() ); - - final BlobRecord record = memoryBlobStore.addRecord( this.segment, source ); - - this.cachedBlobStore = CachedBlobStore.create().blobStore( memoryBlobStore ).memoryCapacity( 100 ).sizeThreshold( 10 ).build(); - - // Cache this - this.cachedBlobStore.getRecord( this.segment, record.getKey() ); - - // Add same record again - final BlobRecord updatedRecord = this.cachedBlobStore.addRecord( this.segment, source ); - - // Only single entry added - try (Stream stream = this.cachedBlobStore.list( this.segment )) { - assertThat(stream).hasSize(1); - } - - final BlobRecord retrievedAfterStore = this.cachedBlobStore.getRecord( this.segment, record.getKey() ); - assertNotNull( retrievedAfterStore ); - assertEquals( updatedRecord.lastModified(), retrievedAfterStore.lastModified() ); - } - @Test public void listSegments() { @@ -193,7 +160,7 @@ public void listSegments() @Test public void deleteSegment() { - final BlobRecord record = newRecord( "0123", 10L ); + final BlobRecord record = newRecord(); assertNull( this.cachedBlobStore.getRecord( segment, record.getKey() ) ); Mockito.when( this.blobStore.getRecord( segment, record.getKey() ) ).thenReturn( record ); assertNotNull( cachedBlobStore.getRecord( segment, record.getKey() ) ); @@ -204,4 +171,36 @@ public void deleteSegment() Mockito.verify( blobStore ).deleteSegment( segment ); assertNull( cachedBlobStore.getRecord( segment, record.getKey() ) ); } + + @Test + void dontCacheCorrupted() { + final BlobRecord corruptedRecord = new BlobRecord() { + + @Override + public BlobKey getKey() + { + return BlobKey.from( "invalidKey" ); + } + + @Override + public long getLength() + { + return 10; + } + + @Override + public ByteSource getBytes() + { + return ByteSource.wrap( new byte[10] ); + } + + @Override + public long lastModified() + { + return 0; + } + }; + cachedBlobStore.addRecord( segment, corruptedRecord ); + assertNull( cachedBlobStore.getRecord( segment, BlobKey.from( "invalidKey" ) ) ); + } } diff --git a/modules/core/core-blobstore/src/testFixtures/java/com/enonic/xp/internal/blobstore/MemoryBlobRecord.java b/modules/core/core-blobstore/src/testFixtures/java/com/enonic/xp/internal/blobstore/MemoryBlobRecord.java index c4da4bcd330..03f6af70759 100644 --- a/modules/core/core-blobstore/src/testFixtures/java/com/enonic/xp/internal/blobstore/MemoryBlobRecord.java +++ b/modules/core/core-blobstore/src/testFixtures/java/com/enonic/xp/internal/blobstore/MemoryBlobRecord.java @@ -18,6 +18,11 @@ public class MemoryBlobRecord private final long lastModified; + public MemoryBlobRecord( final ByteSource source ) + { + this( BlobKey.from( source ), source ); + } + public MemoryBlobRecord( final BlobKey blobKey, final ByteSource source ) { this.blobKey = blobKey; diff --git a/modules/core/core-blobstore/src/testFixtures/java/com/enonic/xp/internal/blobstore/MemoryBlobStore.java b/modules/core/core-blobstore/src/testFixtures/java/com/enonic/xp/internal/blobstore/MemoryBlobStore.java index 60aca7cee11..0d2f5926cb2 100644 --- a/modules/core/core-blobstore/src/testFixtures/java/com/enonic/xp/internal/blobstore/MemoryBlobStore.java +++ b/modules/core/core-blobstore/src/testFixtures/java/com/enonic/xp/internal/blobstore/MemoryBlobStore.java @@ -36,10 +36,9 @@ public BlobRecord getRecord( final Segment segment, final BlobKey key ) public BlobRecord addRecord( final Segment segment, final ByteSource in ) throws BlobStoreException { - final BlobKey key = BlobKey.from( in ); - final MemoryBlobRecord record = new MemoryBlobRecord( key, in ); + final MemoryBlobRecord record = new MemoryBlobRecord( in ); - return doStoreRecord( segment, key, record ); + return doStoreRecord( segment, record.getKey(), record ); } private BlobRecord doStoreRecord( final Segment segment, final BlobKey key, final BlobRecord record ) diff --git a/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/dump/upgrade/htmlarea/HtmlAreaDumpUpgrader.java b/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/dump/upgrade/htmlarea/HtmlAreaDumpUpgrader.java index a532c797e75..d6c7cb8a475 100644 --- a/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/dump/upgrade/htmlarea/HtmlAreaDumpUpgrader.java +++ b/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/dump/upgrade/htmlarea/HtmlAreaDumpUpgrader.java @@ -132,9 +132,9 @@ private PatternIndexConfigDocument getIndexConfigDocument( final DumpBlobRecord private void writeNodeVersion( final NodeVersion nodeVersion, final DumpBlobRecord dumpBlobRecord ) { - final byte[] serializedUpgradedNodeVersion = NodeVersionJsonSerializer.toNodeVersionBytes( nodeVersion ) ; try { + final byte[] serializedUpgradedNodeVersion = NodeVersionJsonSerializer.toNodeVersionBytes( nodeVersion ) ; dumpBlobRecord.override( serializedUpgradedNodeVersion ); } catch ( IOException e ) diff --git a/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/dao/NodeVersionServiceImpl.java b/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/dao/NodeVersionServiceImpl.java index 2fea722238f..70dfbd2530b 100644 --- a/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/dao/NodeVersionServiceImpl.java +++ b/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/dao/NodeVersionServiceImpl.java @@ -1,5 +1,7 @@ package com.enonic.xp.repo.impl.node.dao; +import java.io.IOException; +import java.io.UncheckedIOException; import java.util.List; import java.util.concurrent.ExecutionException; @@ -11,12 +13,10 @@ import com.google.common.cache.CacheBuilder; import com.google.common.cache.Weigher; import com.google.common.io.ByteSource; -import com.google.common.util.concurrent.UncheckedExecutionException; import com.enonic.xp.blob.BlobKey; import com.enonic.xp.blob.BlobRecord; import com.enonic.xp.blob.BlobStore; -import com.enonic.xp.blob.CachingBlobStore; import com.enonic.xp.blob.NodeVersionKey; import com.enonic.xp.blob.Segment; import com.enonic.xp.blob.SegmentLevel; @@ -64,96 +64,98 @@ public NodeVersionServiceImpl( @Reference final BlobStore blobStore, @Reference @Override public NodeVersionKey store( final NodeVersion nodeVersion, final InternalContext context ) { - final Segment nodeSegment = RepositorySegmentUtils.toSegment( context.getRepositoryId(), NodeConstants.NODE_SEGMENT_LEVEL ); - final byte[] nodeJsonString = NodeVersionJsonSerializer.toNodeVersionBytes( nodeVersion ); - final BlobRecord nodeBlobRecord = blobStore.addRecord( nodeSegment, ByteSource.wrap( nodeJsonString ) ); + final RepositoryId repositoryId = context.getRepositoryId(); - final Segment indexConfigSegment = - RepositorySegmentUtils.toSegment( context.getRepositoryId(), NodeConstants.INDEX_CONFIG_SEGMENT_LEVEL ); - final byte[] indexConfigDocumentString = NodeVersionJsonSerializer.toIndexConfigDocumentBytes( nodeVersion ); - final BlobRecord indexConfigBlobRecord = blobStore.addRecord( indexConfigSegment, ByteSource.wrap( indexConfigDocumentString ) ); + final BlobKey accessControlBlobKey = + serializeAndAddBlobRecord( nodeVersion, repositoryId, NodeConstants.ACCESS_CONTROL_SEGMENT_LEVEL, + NodeVersionJsonSerializer::toAccessControlBytes ); + final BlobKey indexConfigBlobKey = serializeAndAddBlobRecord( nodeVersion, repositoryId, NodeConstants.INDEX_CONFIG_SEGMENT_LEVEL, + NodeVersionJsonSerializer::toIndexConfigDocumentBytes ); + final BlobKey nodeBlobKey = serializeAndAddBlobRecord( nodeVersion, repositoryId, NodeConstants.NODE_SEGMENT_LEVEL, + NodeVersionJsonSerializer::toNodeVersionBytes ); - final Segment accessControlSegment = - RepositorySegmentUtils.toSegment( context.getRepositoryId(), NodeConstants.ACCESS_CONTROL_SEGMENT_LEVEL ); - final byte[] accessControlString = NodeVersionJsonSerializer.toAccessControlBytes( nodeVersion ); - final BlobRecord accessControlBlobRecord = blobStore.addRecord( accessControlSegment, ByteSource.wrap( accessControlString ) ); - - return NodeVersionKey.from( nodeBlobRecord.getKey(), indexConfigBlobRecord.getKey(), accessControlBlobRecord.getKey() ); + return NodeVersionKey.from( nodeBlobKey, indexConfigBlobKey, accessControlBlobKey ); } - @Override - public NodeVersion get( final NodeVersionKey nodeVersionKey, final InternalContext context ) + private BlobKey serializeAndAddBlobRecord( final NodeVersion nodeVersion, final RepositoryId repositoryId, final SegmentLevel segmentLevel, + IOFunction serializer ) { - final BlobKey nodeBlobKey = nodeVersionKey.getNodeBlobKey(); - final BlobKey indexConfigBlobKey = nodeVersionKey.getIndexConfigBlobKey(); - final BlobKey accessControlBlobKey = nodeVersionKey.getAccessControlBlobKey(); - + final Segment nodeSegment = RepositorySegmentUtils.toSegment( repositoryId, segmentLevel ); + final byte[] nodeJson; try { - final ImmutableNodeVersion immutableNodeVersion = nodeDataCache.get( nodeBlobKey, () -> { - final BlobRecord nodeBlobRecord = getBlobRecord( NodeConstants.NODE_SEGMENT_LEVEL, context.getRepositoryId(), nodeBlobKey ); - final ByteSource bytes = nodeBlobRecord.getBytes(); - try (var is = bytes.openBufferedStream()) - { - return new WithWeight<>( ImmutableVersionData.deserialize( is ), bytes.size() ); - } - } ).value; - - final PatternIndexConfigDocument indexConfigDocument = indexConfigCache.get( indexConfigBlobKey, () -> { - final BlobRecord indexConfigBlobRecord = - getBlobRecord( NodeConstants.INDEX_CONFIG_SEGMENT_LEVEL, context.getRepositoryId(), indexConfigBlobKey ); - final ByteSource bytes = indexConfigBlobRecord.getBytes(); - return new WithWeight<>( NodeVersionJsonSerializer.toIndexConfigDocument( bytes ), bytes.size() ); - } ).value; - - final NodeVersionAccessControl accessControl = accessControlCache.get( accessControlBlobKey, () -> { - final BlobRecord accessControlBlobRecord = - getBlobRecord( NodeConstants.ACCESS_CONTROL_SEGMENT_LEVEL, context.getRepositoryId(), accessControlBlobKey ); - final ByteSource bytes = accessControlBlobRecord.getBytes(); - return new WithWeight<>( NodeVersionJsonSerializer.toNodeVersionAccessControl( bytes ), bytes.size() ); - } ).value; - - return NodeVersion.create() - .id( immutableNodeVersion.id ) - .nodeType( immutableNodeVersion.nodeType ) - .data( toPropertyTree( immutableNodeVersion.data ) ) - .indexConfigDocument( indexConfigDocument ) - .childOrder( immutableNodeVersion.childOrder ) - .manualOrderValue( immutableNodeVersion.manualOrderValue ) - .attachedBinaries( immutableNodeVersion.attachedBinaries ) - .permissions( accessControl.getPermissions() ) - .inheritPermissions( accessControl.isInheritPermissions() ) - .build(); + nodeJson = serializer.apply( nodeVersion ); } - catch ( ExecutionException | UncheckedExecutionException e ) + catch ( IOException e ) { - if ( blobStore instanceof CachingBlobStore ) - { - ( (CachingBlobStore) blobStore ).invalidate( null, nodeBlobKey ); - ( (CachingBlobStore) blobStore ).invalidate( null, indexConfigBlobKey ); - ( (CachingBlobStore) blobStore ).invalidate( null, accessControlBlobKey ); - } - throw new RuntimeException( - "Failed to load blobs with keys: " + nodeBlobKey + ", " + indexConfigBlobKey + ", " + accessControlBlobKey, e ); + throw new UncheckedIOException( e ); } + return blobStore.addRecord( nodeSegment, ByteSource.wrap( nodeJson ) ).getKey(); } - static PropertyTree toPropertyTree( final List data ) + @Override + public NodeVersion get( final NodeVersionKey nodeVersionKey, final InternalContext context ) + { + final RepositoryId repositoryId = context.getRepositoryId(); + + final NodeVersionAccessControl accessControl = + fetchAndDeserializeCached( repositoryId, NodeConstants.ACCESS_CONTROL_SEGMENT_LEVEL, nodeVersionKey.getAccessControlBlobKey(), + NodeVersionJsonSerializer::toNodeVersionAccessControl, accessControlCache ); + + final PatternIndexConfigDocument indexConfigDocument = + fetchAndDeserializeCached( repositoryId, NodeConstants.INDEX_CONFIG_SEGMENT_LEVEL, nodeVersionKey.getIndexConfigBlobKey(), + NodeVersionJsonSerializer::toIndexConfigDocument, indexConfigCache ); + + final ImmutableNodeVersion immutableNodeVersion = + fetchAndDeserializeCached( repositoryId, NodeConstants.NODE_SEGMENT_LEVEL, nodeVersionKey.getNodeBlobKey(), + ImmutableVersionData::deserialize, nodeDataCache ); + + return NodeVersion.create() + .id( immutableNodeVersion.id ) + .nodeType( immutableNodeVersion.nodeType ) + .data( toPropertyTree( immutableNodeVersion.data ) ) + .indexConfigDocument( indexConfigDocument ) + .childOrder( immutableNodeVersion.childOrder ) + .manualOrderValue( immutableNodeVersion.manualOrderValue ) + .attachedBinaries( immutableNodeVersion.attachedBinaries ) + .permissions( accessControl.getPermissions() ) + .inheritPermissions( accessControl.isInheritPermissions() ) + .build(); + } + + private static PropertyTree toPropertyTree( final List data ) { final PropertyTree result = new PropertyTree(); ImmutableProperty.addToSet( result.getRoot(), data ); return result; } - private BlobRecord getBlobRecord( SegmentLevel segmentLevel, RepositoryId repositoryId, BlobKey blobKey ) + private T fetchAndDeserializeCached( final RepositoryId repositoryId, final SegmentLevel segmentLevel, final BlobKey blobKey, + final IOFunction deserializer, Cache> cache ) { - final Segment nodeSegment = RepositorySegmentUtils.toSegment( repositoryId, segmentLevel ); - final BlobRecord nodeBlobRecord = blobStore.getRecord( nodeSegment, blobKey ); - if ( nodeBlobRecord == null ) + try + { + return cache.get( blobKey, () -> fetchAndDeserialize( repositoryId, segmentLevel, blobKey, deserializer ) ).value; + } + catch ( ExecutionException e ) + { + throw new RuntimeException( String.format( "Failed to load blob %s [%s/%s]", blobKey, repositoryId, segmentLevel ), + e.getCause() ); + } + } + + private WithWeight fetchAndDeserialize( RepositoryId repositoryId, SegmentLevel segmentLevel, BlobKey blobKey, + final IOFunction deserializer ) + throws IOException + { + final Segment segment = RepositorySegmentUtils.toSegment( repositoryId, segmentLevel ); + final BlobRecord blobRecord = blobStore.getRecord( segment, blobKey ); + if ( blobRecord == null ) { - throw new IllegalStateException( "Cannot get node blob with blobKey: " + blobKey + ". Blob is null in segment " + nodeSegment ); + throw new IllegalStateException( String.format( "Blob record not found %s [%s/%s]", blobKey, repositoryId, segmentLevel ) ); } - return nodeBlobRecord; + final ByteSource bytes = blobRecord.getBytes(); + return new WithWeight<>( deserializer.apply( bytes ), blobRecord.getLength() ); } private static class WithWeight @@ -170,4 +172,9 @@ private static class WithWeight static final Weigher> WEIGHTER = ( key, value ) -> value.weight; } + + @FunctionalInterface + private interface IOFunction { + R apply( T t) throws IOException; + } } diff --git a/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/json/ImmutableVersionData.java b/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/json/ImmutableVersionData.java index 6a88da7bcc7..585bc954cdd 100644 --- a/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/json/ImmutableVersionData.java +++ b/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/json/ImmutableVersionData.java @@ -1,7 +1,6 @@ package com.enonic.xp.repo.impl.node.json; import java.io.IOException; -import java.io.InputStream; import java.util.List; import java.util.stream.Collectors; @@ -14,6 +13,7 @@ import com.fasterxml.jackson.databind.JsonDeserializer; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.google.common.io.ByteSource; import com.enonic.xp.data.ValueType; import com.enonic.xp.data.ValueTypes; @@ -39,10 +39,13 @@ private ImmutableVersionData() { } - public static ImmutableNodeVersion deserialize( final InputStream is ) + public static ImmutableNodeVersion deserialize( final ByteSource bytes ) throws IOException { - return OBJECT_MAPPER.readValue( is, ImmutableNodeVersion.class ); + try (var is = bytes.openBufferedStream()) + { + return OBJECT_MAPPER.readValue( is, ImmutableNodeVersion.class ); + } } @JsonIgnoreProperties(ignoreUnknown = true) diff --git a/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/json/NodeVersionJsonSerializer.java b/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/json/NodeVersionJsonSerializer.java index e5135e1e1ec..6e103f12edb 100644 --- a/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/json/NodeVersionJsonSerializer.java +++ b/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/json/NodeVersionJsonSerializer.java @@ -2,9 +2,7 @@ import java.io.IOException; import java.io.InputStream; -import java.io.UncheckedIOException; -import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.io.ByteSource; @@ -17,12 +15,12 @@ public final class NodeVersionJsonSerializer { private static final ObjectMapper MAPPER = ObjectMapperHelper.create(); - public static byte[] toNodeVersionBytes( final NodeVersion nodeVersion ) + public static byte[] toNodeVersionBytes( final NodeVersion nodeVersion ) throws IOException { return writeValueAsBytes( NodeVersionDataJson.toJson( nodeVersion ) ); } - public static byte[] toIndexConfigDocumentBytes( final NodeVersion nodeVersion ) + public static byte[] toIndexConfigDocumentBytes( final NodeVersion nodeVersion ) throws IOException { final IndexConfigDocumentJson entityIndexConfig; final IndexConfigDocument indexConfig = nodeVersion.getIndexConfigDocument(); @@ -37,7 +35,7 @@ public static byte[] toIndexConfigDocumentBytes( final NodeVersion nodeVersion ) return writeValueAsBytes( entityIndexConfig ); } - public static byte[] toAccessControlBytes( final NodeVersion nodeVersion ) + public static byte[] toAccessControlBytes( final NodeVersion nodeVersion ) throws IOException { return writeValueAsBytes( AccessControlJson.toJson( nodeVersion ) ); } @@ -85,15 +83,8 @@ private static T readValue( final ByteSource src, final Class valueType ) } } - private static byte[] writeValueAsBytes( Object object ) + private static byte[] writeValueAsBytes( Object object ) throws IOException { - try - { - return MAPPER.writeValueAsBytes( object ); - } - catch ( JsonProcessingException e ) - { - throw new UncheckedIOException( e ); - } + return MAPPER.writeValueAsBytes( object ); } } diff --git a/modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/node/dao/NodeVersionServiceImplTest.java b/modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/node/dao/NodeVersionServiceImplTest.java index 89d4cac5ad9..e9abfcfa825 100644 --- a/modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/node/dao/NodeVersionServiceImplTest.java +++ b/modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/node/dao/NodeVersionServiceImplTest.java @@ -27,7 +27,6 @@ import com.enonic.xp.index.PatternIndexConfigDocument; import com.enonic.xp.internal.blobstore.MemoryBlobRecord; import com.enonic.xp.internal.blobstore.MemoryBlobStore; -import com.enonic.xp.internal.blobstore.cache.CachedBlobStore; import com.enonic.xp.json.ObjectMapperHelper; import com.enonic.xp.node.NodeId; import com.enonic.xp.node.NodeType; @@ -219,15 +218,13 @@ public void getVersionCorrupted() BLOB_STORE.addRecord( segment, corruptedBlob ); RuntimeException e = assertThrows( RuntimeException.class, () -> nodeDao.get( nodeVersionKey, createInternalContext() ) ); - assertTrue( e.getMessage().startsWith( "Failed to load blobs with keys" ) ); + assertTrue( e.getMessage().startsWith( "Failed to load blob" ) ); } @Test public void avoidCachingVersionCorrupted() throws Exception { - final CachedBlobStore cachedBlobStore = CachedBlobStore.create().blobStore( BLOB_STORE ).build(); - final PropertyTree data = new PropertyTree(); data.addString( "myName", "myValue" ); @@ -248,10 +245,9 @@ public void avoidCachingVersionCorrupted() final byte[] blobDataTruncated = Arrays.copyOf( blobData, blobData.length / 2 ); final MemoryBlobRecord corruptedBlob = new MemoryBlobRecord( blob.getKey(), ByteSource.wrap( blobDataTruncated ) ); BLOB_STORE.addRecord( segment, corruptedBlob ); - cachedBlobStore.invalidate( segment, blob.getKey() ); RuntimeException e = assertThrows( RuntimeException.class, () -> nodeDao.get( nodeVersionKey, createInternalContext() ) ); - assertTrue( e.getMessage().startsWith( "Failed to load blobs with keys" ) ); + assertTrue( e.getMessage().startsWith( "Failed to load blob" ) ); // restore original blob in source blob store BLOB_STORE.addRecord( segment, blob ); diff --git a/modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/vacuum/blob/AbstractBlobVacuumTaskTest.java b/modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/vacuum/blob/AbstractBlobVacuumTaskTest.java index af3d62d9772..1433f286412 100644 --- a/modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/vacuum/blob/AbstractBlobVacuumTaskTest.java +++ b/modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/vacuum/blob/AbstractBlobVacuumTaskTest.java @@ -5,14 +5,12 @@ import org.mockito.Mockito; -import com.google.common.base.Strings; import com.google.common.io.ByteSource; import com.enonic.xp.blob.BlobKey; import com.enonic.xp.blob.BlobStore; import com.enonic.xp.blob.Segment; import com.enonic.xp.data.ValueFactory; -import com.enonic.xp.internal.blobstore.MemoryBlobRecord; import com.enonic.xp.internal.blobstore.MemoryBlobStore; import com.enonic.xp.node.NodeService; import com.enonic.xp.node.NodeVersionQuery; @@ -43,7 +41,7 @@ public void setUp() thenAnswer( ( invocation ) -> { final NodeVersionQuery query = invocation.getArgument( 0 ); final ValueFilter valueFilter = (ValueFilter) query.getQueryFilters().first(); - if ( valueFilter.getValues().contains( ValueFactory.newString( createBlobKey( 'a' ).toString() ) ) ) + if ( valueFilter.getValues().contains( ValueFactory.newString( BlobKey.from( ByteSource.wrap( "a-stuff".getBytes() ) ).toString() ) ) ) { return NodeVersionQueryResult.empty( 1 ); } @@ -54,9 +52,9 @@ public void setUp() public void test_delete_unused() throws Exception { - this.blobStore.addRecord( segment, createBlobRecord( 'a' ) ); - this.blobStore.addRecord( segment, createBlobRecord( 'b' ) ); - this.blobStore.addRecord( segment, createBlobRecord( 'c' ) ); + this.blobStore.addRecord( segment, ByteSource.wrap( "a-stuff".getBytes() ) ); + this.blobStore.addRecord( segment, ByteSource.wrap( "b-stuff".getBytes() ) ); + this.blobStore.addRecord( segment, ByteSource.wrap( "c-stuff".getBytes() ) ); final VacuumTask task = createTask(); @@ -70,9 +68,9 @@ public void test_delete_unused() public void test_progress_report() throws Exception { - this.blobStore.addRecord( segment, createBlobRecord( 'a' ) ); - this.blobStore.addRecord( segment, createBlobRecord( 'b' ) ); - this.blobStore.addRecord( segment, createBlobRecord( 'c' ) ); + this.blobStore.addRecord( segment, ByteSource.wrap( "a-stuff".getBytes() ) ); + this.blobStore.addRecord( segment, ByteSource.wrap( "b-stuff".getBytes() ) ); + this.blobStore.addRecord( segment, ByteSource.wrap( "c-stuff".getBytes() ) ); final VacuumTask task = createTask(); @@ -115,7 +113,7 @@ public void processed( final long count ) public void age_threshold() throws Exception { - this.blobStore.addRecord( segment, createBlobRecord( 'a' ) ); + this.blobStore.addRecord( segment, ByteSource.wrap( "a-stuff".getBytes() ) ); final VacuumTask task = createTask(); final VacuumTaskResult result = task.execute( VacuumTaskParams.create().vacuumStartedAt( Instant.now() ).build() ); @@ -124,14 +122,4 @@ public void age_threshold() } protected abstract VacuumTask createTask(); - - private MemoryBlobRecord createBlobRecord( final char id ) - { - return new MemoryBlobRecord( createBlobKey( id ), ByteSource.wrap( "stuff".getBytes() ) ); - } - - private BlobKey createBlobKey( final char value ) - { - return BlobKey.from( Strings.padStart( "", 40, value ) ); - } }