From e069627691fe99ff88194e63e18762c40bb7ef7e Mon Sep 17 00:00:00 2001 From: Anatol Sialitski Date: Thu, 12 Sep 2019 15:41:48 +0300 Subject: [PATCH 1/4] Content Revert - Content Resource #7388 - implemented --- .../resource/content/ContentResource.java | 35 ++++++++ .../content/json/RevertContentJson.java | 31 +++++++ .../resource/content/ContentResourceTest.java | 85 ++++++++++++++++++- 3 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/json/RevertContentJson.java diff --git a/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java b/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java index a05643b4a8a..dc2831f31ec 100644 --- a/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java +++ b/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java @@ -96,6 +96,7 @@ import com.enonic.xp.admin.impl.rest.resource.content.json.ReorderChildrenJson; import com.enonic.xp.admin.impl.rest.resource.content.json.ResolvePublishContentResultJson; import com.enonic.xp.admin.impl.rest.resource.content.json.ResolvePublishDependenciesJson; +import com.enonic.xp.admin.impl.rest.resource.content.json.RevertContentJson; import com.enonic.xp.admin.impl.rest.resource.content.json.SetActiveVersionJson; import com.enonic.xp.admin.impl.rest.resource.content.json.SetChildOrderJson; import com.enonic.xp.admin.impl.rest.resource.content.json.UndoPendingDeleteContentJson; @@ -134,6 +135,7 @@ import com.enonic.xp.content.ContentService; import com.enonic.xp.content.ContentValidityParams; import com.enonic.xp.content.ContentValidityResult; +import com.enonic.xp.content.ContentVersionId; import com.enonic.xp.content.Contents; import com.enonic.xp.content.CreateMediaParams; import com.enonic.xp.content.FindContentByParentParams; @@ -1417,6 +1419,39 @@ public List listChildrenIds( @QueryParam("parentId") final String return result.getContentIds().stream().map( contentId -> new ContentIdJson( contentId ) ).collect( Collectors.toList() ); } + @POST + @Path("revert") + public ContentJson revert( final RevertContentJson params ) + { + final ContentVersionId contentVersionId = ContentVersionId.from( params.getVersionId() ); + + final Content content = + params.getContentKey().startsWith( "/" ) + ? contentService.getByPathAndVersionId( ContentPath.from( params.getContentKey() ), contentVersionId ) + : contentService.getByIdAndVersionId( ContentId.from( params.getContentKey() ), contentVersionId ); + + if ( content == null ) + { + throw JaxRsExceptions.notFound( "Content with contentKey [%s] and versionId [%s] not found", params.getContentKey(), + params.getVersionId() ); + } + + final UpdateContentParams updateParams = new UpdateContentParams(). + contentId( content.getId() ). + editor( edit -> { + edit.data = content.getData(); + edit.extraDatas = content.getAllExtraData(); + edit.displayName = content.getDisplayName(); + edit.owner = content.getOwner(); + edit.language = content.getLanguage(); + edit.workflowInfo = WorkflowInfo.inProgress(); + } ); + + final Content updatedContent = contentService.update( updateParams ); + + return new ContentJson( updatedContent, contentIconUrlResolver, principalsResolver ); + } + private Content doCreateAttachment( final String attachmentName, final MultipartForm form ) { final MultipartItem mediaFile = form.get( "file" ); diff --git a/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/json/RevertContentJson.java b/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/json/RevertContentJson.java new file mode 100644 index 00000000000..138561612ea --- /dev/null +++ b/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/json/RevertContentJson.java @@ -0,0 +1,31 @@ +package com.enonic.xp.admin.impl.rest.resource.content.json; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; + +public class RevertContentJson +{ + + private final String contentKey; + + private final String versionId; + + @JsonCreator + public RevertContentJson( final @JsonProperty(value = "contentKey", required = true) String contentKey, + final @JsonProperty(value = "versionId", required = true) String versionId ) + { + this.contentKey = contentKey; + this.versionId = versionId; + } + + public String getContentKey() + { + return contentKey; + } + + public String getVersionId() + { + return versionId; + } + +} diff --git a/modules/admin/admin-impl/src/test/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResourceTest.java b/modules/admin/admin-impl/src/test/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResourceTest.java index c7311b1c66b..a4e096a9abe 100644 --- a/modules/admin/admin-impl/src/test/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResourceTest.java +++ b/modules/admin/admin-impl/src/test/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResourceTest.java @@ -66,6 +66,7 @@ import com.enonic.xp.admin.impl.rest.resource.content.json.MoveContentJson; import com.enonic.xp.admin.impl.rest.resource.content.json.PublishContentJson; import com.enonic.xp.admin.impl.rest.resource.content.json.ReorderChildrenJson; +import com.enonic.xp.admin.impl.rest.resource.content.json.RevertContentJson; import com.enonic.xp.admin.impl.rest.resource.content.json.SetActiveVersionJson; import com.enonic.xp.admin.impl.rest.resource.content.json.UndoPendingDeleteContentJson; import com.enonic.xp.admin.impl.rest.resource.content.json.UndoPendingDeleteContentResultJson; @@ -192,7 +193,11 @@ import static com.enonic.xp.security.acl.Permission.MODIFY; import static com.enonic.xp.security.acl.Permission.READ; import static java.util.Arrays.asList; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; @@ -2218,6 +2223,84 @@ public void testLoadImageWithMalformedUrl() } ); } + @Test + public void testRevert_by_id() + { + // prepare + final ContentResource instance = getResourceInstance(); + final RevertContentJson params = new RevertContentJson( "content-id", "versionKey" ); + final Content updatedContent = createContent( "content-id", "content-name", "myapplication:content-type" ); + + // mock + final Content content = Mockito.mock( Content.class ); + Mockito.when( contentService.getByIdAndVersionId( any( ContentId.class ), any( ContentVersionId.class ) ) ).thenReturn( content ); + Mockito.when( contentService.update( any( UpdateContentParams.class ) ) ).thenReturn( updatedContent ); + + // test + final ContentJson result = instance.revert( params ); + + // assert + assertNotNull( result ); + assertEquals( "content-id", result.getId() ); + + // verify + Mockito.verify( this.contentService, Mockito.times( 1 ) ). + getByIdAndVersionId( any( ContentId.class ), any( ContentVersionId.class ) ); + Mockito.verify( this.contentService, Mockito.times( 1 ) ). + update( any( UpdateContentParams.class ) ); + Mockito.verifyNoMoreInteractions( contentService ); + } + + @Test + public void testRevert_by_path() + { + // prepare + final ContentResource instance = getResourceInstance(); + final RevertContentJson params = new RevertContentJson( "/content-name", "versionKey" ); + final Content updatedContent = createContent( "content-id", "content-name", "myapplication:content-type" ); + + // mock + final Content content = Mockito.mock( Content.class ); + Mockito.when( contentService.getByPathAndVersionId( any( ContentPath.class ), any( ContentVersionId.class ) ) ).thenReturn( content ); + Mockito.when( contentService.update( any( UpdateContentParams.class ) ) ).thenReturn( updatedContent ); + + // test + final ContentJson result = instance.revert( params ); + + // assert + assertNotNull( result ); + assertEquals( "content-id", result.getId() ); + assertEquals( "/content-name", result.getPath() ); + + // verify + Mockito.verify( this.contentService, Mockito.times( 1 ) ). + getByPathAndVersionId( any( ContentPath.class ), any( ContentVersionId.class ) ); + Mockito.verify( this.contentService, Mockito.times( 1 ) ). + update( any( UpdateContentParams.class ) ); + Mockito.verifyNoMoreInteractions( contentService ); + } + + @Test + public void testRevert_not_found() + { + // prepare + final ContentResource instance = getResourceInstance(); + final RevertContentJson params = new RevertContentJson( "/content-name", "versionKey" ); + + // mock + Mockito.when( contentService.getByPathAndVersionId( any( ContentPath.class ), any( ContentVersionId.class ) ) ).thenReturn( null ); + + // test & assert + final WebApplicationException exception = assertThrows(WebApplicationException.class, () -> instance.revert( params )); + + assertEquals( "Content with contentKey [/content-name] and versionId [versionKey] not found", exception.getMessage() ); + + // verify + Mockito.verify( this.contentService, Mockito.times( 1 ) ). + getByPathAndVersionId( any( ContentPath.class ), any( ContentVersionId.class ) ); + Mockito.verifyNoMoreInteractions( contentService ); + } + private ContentTreeSelectorQueryJson initContentTreeSelectorQueryJson( final ContentPath parentPath ) { final ContentTreeSelectorQueryJson json = Mockito.mock( ContentTreeSelectorQueryJson.class ); From 8bef8cc805b108a707ab39a27cc597aaee668b42 Mon Sep 17 00:00:00 2001 From: Anatol Sialitski Date: Fri, 27 Sep 2019 17:50:30 +0300 Subject: [PATCH 2/4] The endpoint /admin/rest/content/image/ does not work after update of binary source of media content when the revert action is triggered #7470 -fixed --- .../resource/content/ContentResource.java | 56 +++++++++++++ .../com/enonic/xp/content/ContentService.java | 2 + .../core/impl/content/ContentServiceImpl.java | 12 +++ .../content/GetBinaryByVersionCommand.java | 83 +++++++++++++++++++ 4 files changed, 153 insertions(+) create mode 100644 modules/core/core-content/src/main/java/com/enonic/xp/core/impl/content/GetBinaryByVersionCommand.java diff --git a/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java b/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java index dc2831f31ec..571407402b2 100644 --- a/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java +++ b/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java @@ -112,7 +112,9 @@ import com.enonic.xp.admin.impl.rest.resource.content.task.UnpublishRunnableTask; import com.enonic.xp.admin.impl.rest.resource.schema.content.ContentTypeIconResolver; import com.enonic.xp.admin.impl.rest.resource.schema.content.ContentTypeIconUrlResolver; +import com.enonic.xp.attachment.Attachment; import com.enonic.xp.attachment.AttachmentNames; +import com.enonic.xp.attachment.Attachments; import com.enonic.xp.attachment.CreateAttachment; import com.enonic.xp.attachment.CreateAttachments; import com.enonic.xp.branch.Branches; @@ -1447,11 +1449,65 @@ public ContentJson revert( final RevertContentJson params ) edit.workflowInfo = WorkflowInfo.inProgress(); } ); + updateContentParamsAttachments( content, contentVersionId, updateParams ); + final Content updatedContent = contentService.update( updateParams ); return new ContentJson( updatedContent, contentIconUrlResolver, principalsResolver ); } + + private void updateContentParamsAttachments( final Content content, final ContentVersionId contentVersionId, + final UpdateContentParams updateParams ) + { + if ( content.getAttachments() == null ) + { + return; + } + + updateParams.clearAttachments( true ); + updateParams.createAttachments( createAttachments( content.getId(), contentVersionId, content.getAttachments() ) ); + } + + private CreateAttachments createAttachments( final ContentId contentId, final ContentVersionId contentVersionId, + final Attachments attachments ) + { + final CreateAttachments.Builder createBuilder = CreateAttachments.create(); + + attachments.forEach( attachment -> { + final ByteSource sourceBinary = contentService.getBinary( contentId, contentVersionId, attachment.getBinaryReference() ); + + if ( sourceBinary != null ) + { + final CreateAttachment createAttachment = createAttachment( contentId, contentVersionId, attachment ); + if ( createAttachment != null ) + { + createBuilder.add( createAttachment ); + } + } + } ); + + return createBuilder.build(); + } + + private CreateAttachment createAttachment( final ContentId contentId, final ContentVersionId contentVersionId, + final Attachment sourceAttachment ) + { + final ByteSource sourceBinary = contentService.getBinary( contentId, contentVersionId, sourceAttachment.getBinaryReference() ); + + if ( sourceBinary != null ) + { + return CreateAttachment.create(). + name( sourceAttachment.getName() ). + mimeType( sourceAttachment.getMimeType() ). + byteSource( sourceBinary ). + text( sourceAttachment.getTextContent() ). + build(); + } + + return null; + } + private Content doCreateAttachment( final String attachmentName, final MultipartForm form ) { final MultipartItem mediaFile = form.get( "file" ); diff --git a/modules/core/core-api/src/main/java/com/enonic/xp/content/ContentService.java b/modules/core/core-api/src/main/java/com/enonic/xp/content/ContentService.java index 0e5e2cec345..df89c30798f 100644 --- a/modules/core/core-api/src/main/java/com/enonic/xp/content/ContentService.java +++ b/modules/core/core-api/src/main/java/com/enonic/xp/content/ContentService.java @@ -105,6 +105,8 @@ public interface ContentService ByteSource getBinary( ContentId contentId, BinaryReference binaryReference ); + ByteSource getBinary( ContentId contentId, ContentVersionId contentVersionId, BinaryReference binaryReference ); + @Deprecated InputStream getBinaryInputStream( ContentId contentId, BinaryReference binaryReference ); diff --git a/modules/core/core-content/src/main/java/com/enonic/xp/core/impl/content/ContentServiceImpl.java b/modules/core/core-content/src/main/java/com/enonic/xp/core/impl/content/ContentServiceImpl.java index 8faec0e9344..206fe849e6b 100644 --- a/modules/core/core-content/src/main/java/com/enonic/xp/core/impl/content/ContentServiceImpl.java +++ b/modules/core/core-content/src/main/java/com/enonic/xp/core/impl/content/ContentServiceImpl.java @@ -1053,6 +1053,18 @@ public ByteSource getBinary( final ContentId contentId, final BinaryReference bi execute(); } + @Override + public ByteSource getBinary( final ContentId contentId, final ContentVersionId contentVersionId, final BinaryReference binaryReference ) + { + return GetBinaryByVersionCommand.create( contentId, contentVersionId, binaryReference ). + nodeService( this.nodeService ). + contentTypeService( this.contentTypeService ). + translator( this.translator ). + eventPublisher( this.eventPublisher ). + build(). + execute(); + } + @Override public String getBinaryKey( final ContentId contentId, final BinaryReference binaryReference ) { diff --git a/modules/core/core-content/src/main/java/com/enonic/xp/core/impl/content/GetBinaryByVersionCommand.java b/modules/core/core-content/src/main/java/com/enonic/xp/core/impl/content/GetBinaryByVersionCommand.java new file mode 100644 index 00000000000..74cfe88682e --- /dev/null +++ b/modules/core/core-content/src/main/java/com/enonic/xp/core/impl/content/GetBinaryByVersionCommand.java @@ -0,0 +1,83 @@ +package com.enonic.xp.core.impl.content; + +import com.google.common.io.ByteSource; + +import com.enonic.xp.content.ContentId; +import com.enonic.xp.content.ContentVersionId; +import com.enonic.xp.node.NodeId; +import com.enonic.xp.node.NodeVersionId; +import com.enonic.xp.util.BinaryReference; + +public class GetBinaryByVersionCommand + extends AbstractContentCommand +{ + private final ContentId contentId; + + private final ContentVersionId contentVersionId; + + private final BinaryReference binaryReference; + + private GetBinaryByVersionCommand( final Builder builder ) + { + super( builder ); + this.contentId = builder.contentId; + this.contentVersionId = builder.contentVersionId; + this.binaryReference = builder.binaryReference; + } + + public ByteSource execute() + { + return nodeService.getBinary( NodeId.from( contentId.toString() ), NodeVersionId.from( contentVersionId.toString() ), + binaryReference ); + } + + public static Builder create( final ContentId contentId, final ContentVersionId contentVersionId, + final BinaryReference binaryReference ) + { + return new Builder( contentId, contentVersionId, binaryReference ); + } + + public static class Builder + extends AbstractContentCommand.Builder + { + + private ContentId contentId; + + private ContentVersionId contentVersionId; + + private BinaryReference binaryReference; + + public Builder( final ContentId contentId, final ContentVersionId contentVersionId, final BinaryReference binaryReference ) + { + super(); + this.contentId = contentId; + this.contentVersionId = contentVersionId; + this.binaryReference = binaryReference; + } + + public Builder contentId( final ContentId contentId ) + { + this.contentId = contentId; + return this; + } + + public Builder contentVersionId( final ContentVersionId contentVersionId ) + { + this.contentVersionId = contentVersionId; + return this; + } + + public Builder binaryReference( final BinaryReference binaryReference ) + { + this.binaryReference = binaryReference; + return this; + } + + public GetBinaryByVersionCommand build() + { + return new GetBinaryByVersionCommand( this ); + } + + } + +} From a0b7b4b2534274f177edf58333763079d9a4b9cc Mon Sep 17 00:00:00 2001 From: Anatol Sialitski Date: Sat, 28 Sep 2019 10:51:33 +0300 Subject: [PATCH 3/4] The endpoint /admin/rest/content/image/ does not work after update of binary source of media content when the revert action is triggered #7470 -added tests --- .../resource/content/ContentResource.java | 57 +++++++++---------- .../resource/content/ContentResourceTest.java | 13 +++++ .../node/GetBinaryByVersionCommandTest.java | 54 ++++++++++++++++++ 3 files changed, 94 insertions(+), 30 deletions(-) create mode 100644 modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/node/GetBinaryByVersionCommandTest.java diff --git a/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java b/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java index 571407402b2..b8512f38fdc 100644 --- a/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java +++ b/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java @@ -1427,46 +1427,45 @@ public ContentJson revert( final RevertContentJson params ) { final ContentVersionId contentVersionId = ContentVersionId.from( params.getVersionId() ); - final Content content = + final Content versionedContent = params.getContentKey().startsWith( "/" ) ? contentService.getByPathAndVersionId( ContentPath.from( params.getContentKey() ), contentVersionId ) : contentService.getByIdAndVersionId( ContentId.from( params.getContentKey() ), contentVersionId ); - if ( content == null ) + if ( versionedContent == null ) { throw JaxRsExceptions.notFound( "Content with contentKey [%s] and versionId [%s] not found", params.getContentKey(), params.getVersionId() ); } - final UpdateContentParams updateParams = new UpdateContentParams(). - contentId( content.getId() ). - editor( edit -> { - edit.data = content.getData(); - edit.extraDatas = content.getAllExtraData(); - edit.displayName = content.getDisplayName(); - edit.owner = content.getOwner(); - edit.language = content.getLanguage(); - edit.workflowInfo = WorkflowInfo.inProgress(); - } ); + final UpdateContentParams updateParams = prepareUpdateContentParams( versionedContent, contentVersionId ); - updateContentParamsAttachments( content, contentVersionId, updateParams ); + final Content content = contentService.update( updateParams ); - final Content updatedContent = contentService.update( updateParams ); - - return new ContentJson( updatedContent, contentIconUrlResolver, principalsResolver ); + return new ContentJson( content, contentIconUrlResolver, principalsResolver ); } - private void updateContentParamsAttachments( final Content content, final ContentVersionId contentVersionId, - final UpdateContentParams updateParams ) + private UpdateContentParams prepareUpdateContentParams( final Content versionedContent, final ContentVersionId contentVersionId ) { - if ( content.getAttachments() == null ) + final UpdateContentParams updateParams = new UpdateContentParams(). + contentId( versionedContent.getId() ). + editor( edit -> { + edit.data = versionedContent.getData(); + edit.extraDatas = versionedContent.getAllExtraData(); + edit.displayName = versionedContent.getDisplayName(); + edit.owner = versionedContent.getOwner(); + edit.language = versionedContent.getLanguage(); + edit.workflowInfo = WorkflowInfo.inProgress(); + } ); + + if ( versionedContent.getAttachments() != null ) { - return; + updateParams.clearAttachments( true ); + updateParams.createAttachments( createAttachments( versionedContent.getId(), contentVersionId, versionedContent.getAttachments() ) ); } - updateParams.clearAttachments( true ); - updateParams.createAttachments( createAttachments( content.getId(), contentVersionId, content.getAttachments() ) ); + return updateParams; } private CreateAttachments createAttachments( final ContentId contentId, final ContentVersionId contentVersionId, @@ -1475,19 +1474,17 @@ private CreateAttachments createAttachments( final ContentId contentId, final Co final CreateAttachments.Builder createBuilder = CreateAttachments.create(); attachments.forEach( attachment -> { - final ByteSource sourceBinary = contentService.getBinary( contentId, contentVersionId, attachment.getBinaryReference() ); + final CreateAttachment createAttachment = createAttachment( contentId, contentVersionId, attachment ); - if ( sourceBinary != null ) + if ( createAttachment != null ) { - final CreateAttachment createAttachment = createAttachment( contentId, contentVersionId, attachment ); - if ( createAttachment != null ) - { - createBuilder.add( createAttachment ); - } + createBuilder.add( createAttachment ); } } ); - return createBuilder.build(); + final CreateAttachments createAttachments = createBuilder.build(); + + return createAttachments.isNotEmpty() ? createAttachments : null; } private CreateAttachment createAttachment( final ContentId contentId, final ContentVersionId contentVersionId, diff --git a/modules/admin/admin-impl/src/test/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResourceTest.java b/modules/admin/admin-impl/src/test/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResourceTest.java index a4e096a9abe..ca0fa335ae6 100644 --- a/modules/admin/admin-impl/src/test/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResourceTest.java +++ b/modules/admin/admin-impl/src/test/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResourceTest.java @@ -183,6 +183,7 @@ import com.enonic.xp.task.TaskId; import com.enonic.xp.task.TaskResultJson; import com.enonic.xp.task.TaskService; +import com.enonic.xp.util.BinaryReference; import com.enonic.xp.util.BinaryReferences; import com.enonic.xp.web.HttpStatus; import com.enonic.xp.web.multipart.MultipartForm; @@ -2233,8 +2234,18 @@ public void testRevert_by_id() // mock final Content content = Mockito.mock( Content.class ); + final ByteSource byteSource = Mockito.mock( ByteSource.class ); + + final Attachments attachments = Attachments.create().add( Attachment.create(). + name( "attachment" ).mimeType( "mimeType" ).size( 1000L ).build() ).build(); + + Mockito.when( content.getId() ).thenReturn( ContentId.from( "nodeId" ) ); + Mockito.when( content.getAttachments() ).thenReturn( attachments ); Mockito.when( contentService.getByIdAndVersionId( any( ContentId.class ), any( ContentVersionId.class ) ) ).thenReturn( content ); Mockito.when( contentService.update( any( UpdateContentParams.class ) ) ).thenReturn( updatedContent ); + Mockito.when( + contentService.getBinary( any( ContentId.class ), any( ContentVersionId.class ), any( BinaryReference.class ) ) ).thenReturn( + byteSource ); // test final ContentJson result = instance.revert( params ); @@ -2246,6 +2257,8 @@ public void testRevert_by_id() // verify Mockito.verify( this.contentService, Mockito.times( 1 ) ). getByIdAndVersionId( any( ContentId.class ), any( ContentVersionId.class ) ); + Mockito.verify( this.contentService, Mockito.times( 1 ) ). + getBinary( any( ContentId.class ), any( ContentVersionId.class ), any( BinaryReference.class ) ); Mockito.verify( this.contentService, Mockito.times( 1 ) ). update( any( UpdateContentParams.class ) ); Mockito.verifyNoMoreInteractions( contentService ); diff --git a/modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/node/GetBinaryByVersionCommandTest.java b/modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/node/GetBinaryByVersionCommandTest.java new file mode 100644 index 00000000000..44197657d8c --- /dev/null +++ b/modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/node/GetBinaryByVersionCommandTest.java @@ -0,0 +1,54 @@ +package com.enonic.xp.repo.impl.node; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import com.google.common.io.ByteSource; + +import com.enonic.xp.data.PropertyTree; +import com.enonic.xp.node.CreateNodeParams; +import com.enonic.xp.node.Node; +import com.enonic.xp.node.NodePath; +import com.enonic.xp.util.BinaryReference; + +import static org.junit.jupiter.api.Assertions.assertNotNull; + +public class GetBinaryByVersionCommandTest + extends AbstractNodeTest +{ + @BeforeEach + public void setUp() + throws Exception + { + this.createDefaultRootNode(); + } + + @Test + public void testExecute() + { + final PropertyTree data = new PropertyTree(); + final BinaryReference imageRef = BinaryReference.from( "myImage" ); + + data.addBinaryReferences( "myBinary", imageRef ); + + final Node node = createNode( CreateNodeParams.create(). + name( "my-node" ). + parent( NodePath.ROOT ). + data( data ). + attachBinary( imageRef, ByteSource.wrap( "thisIsMyImage".getBytes() ) ). + build() ); + + final ByteSource myImage = GetBinaryByVersionCommand.create(). + nodeId( node.id() ). + nodeVersionId( node.getNodeVersionId() ). + binaryReference( imageRef ). + indexServiceInternal( this.indexServiceInternal ). + binaryService( this.binaryService ). + storageService( this.storageService ). + searchService( this.searchService ). + build(). + execute(); + + assertNotNull( myImage ); + } +} From bb0300be66dd0a2d9be561af6945c5c24b738ede Mon Sep 17 00:00:00 2001 From: Anatol Sialitski Date: Mon, 7 Oct 2019 15:02:57 +0300 Subject: [PATCH 4/4] Give a different message when a version was not reverted #7487 - return ContentVersionJson instead of ContentJson for the revert action - remove and create attachments if needed, to exclude a creation new a version of content --- .../resource/content/ContentResource.java | 55 ++++++++++++++++--- .../resource/content/ContentResourceTest.java | 41 +++++++++++--- .../com/enonic/xp/content/ContentService.java | 2 + .../core/impl/content/ContentServiceImpl.java | 14 +++++ 4 files changed, 95 insertions(+), 17 deletions(-) diff --git a/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java b/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java index b8512f38fdc..b51ea13528a 100644 --- a/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java +++ b/modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResource.java @@ -53,6 +53,7 @@ import com.enonic.xp.admin.impl.json.content.ContentSummaryJson; import com.enonic.xp.admin.impl.json.content.ContentSummaryListJson; import com.enonic.xp.admin.impl.json.content.ContentTreeSelectorListJson; +import com.enonic.xp.admin.impl.json.content.ContentVersionJson; import com.enonic.xp.admin.impl.json.content.ContentsExistByPathJson; import com.enonic.xp.admin.impl.json.content.ContentsExistJson; import com.enonic.xp.admin.impl.json.content.DependenciesAggregationJson; @@ -137,6 +138,7 @@ import com.enonic.xp.content.ContentService; import com.enonic.xp.content.ContentValidityParams; import com.enonic.xp.content.ContentValidityResult; +import com.enonic.xp.content.ContentVersion; import com.enonic.xp.content.ContentVersionId; import com.enonic.xp.content.Contents; import com.enonic.xp.content.CreateMediaParams; @@ -187,12 +189,14 @@ import com.enonic.xp.security.auth.AuthenticationInfo; import com.enonic.xp.task.TaskResultJson; import com.enonic.xp.task.TaskService; +import com.enonic.xp.util.BinaryReference; import com.enonic.xp.util.Exceptions; import com.enonic.xp.web.HttpStatus; import com.enonic.xp.web.multipart.MultipartForm; import com.enonic.xp.web.multipart.MultipartItem; import static java.lang.Math.toIntExact; +import static java.util.Optional.ofNullable; import static org.apache.commons.lang.StringUtils.containsIgnoreCase; import static org.apache.commons.lang.StringUtils.isBlank; @@ -1423,7 +1427,7 @@ public List listChildrenIds( @QueryParam("parentId") final String @POST @Path("revert") - public ContentJson revert( final RevertContentJson params ) + public ContentVersionJson revert( final RevertContentJson params ) { final ContentVersionId contentVersionId = ContentVersionId.from( params.getVersionId() ); @@ -1438,11 +1442,19 @@ public ContentJson revert( final RevertContentJson params ) params.getVersionId() ); } - final UpdateContentParams updateParams = prepareUpdateContentParams( versionedContent, contentVersionId ); + final Content revertedContent = contentService.update( prepareUpdateContentParams( versionedContent, contentVersionId ) ); - final Content content = contentService.update( updateParams ); + final ContentVersion contentVersion = contentService.getActiveVersion( GetActiveContentVersionsParams.create(). + branches( Branches.from( ContentConstants.BRANCH_DRAFT ) ). + contentId( revertedContent.getId() ). + build() ); - return new ContentJson( content, contentIconUrlResolver, principalsResolver ); + if ( contentVersion != null ) + { + return new ContentVersionJson( contentVersion, principalsResolver ); + } + + return null; } @@ -1459,13 +1471,40 @@ private UpdateContentParams prepareUpdateContentParams( final Content versionedC edit.workflowInfo = WorkflowInfo.inProgress(); } ); - if ( versionedContent.getAttachments() != null ) + updateAttachments( versionedContent, contentVersionId, updateParams ); + + return updateParams; + } + + private void updateAttachments( final Content versionedContent, final ContentVersionId contentVersionId, + final UpdateContentParams updateParams ) + { + final Content content = contentService.getById( versionedContent.getId() ); + + final List sourceAttachments = + ofNullable( content.getAttachments() ).orElse( Attachments.empty() ).stream().map( Attachment::getBinaryReference ).collect( + Collectors.toList() ); + + final List targetAttachments = + ofNullable( versionedContent.getAttachments() ).orElse( Attachments.empty() ).stream().map( + Attachment::getBinaryReference ).collect( Collectors.toList() ); + + List difference; + if ( sourceAttachments.size() > targetAttachments.size() ) { - updateParams.clearAttachments( true ); - updateParams.createAttachments( createAttachments( versionedContent.getId(), contentVersionId, versionedContent.getAttachments() ) ); + difference = sourceAttachments.stream().filter( ref -> !targetAttachments.contains( ref ) ).collect( Collectors.toList() ); + } + else + { + difference = targetAttachments.stream().filter( ref -> !sourceAttachments.contains( ref ) ).collect( Collectors.toList() ); } - return updateParams; + if ( !difference.isEmpty() ) + { + updateParams.clearAttachments( true ); + updateParams.createAttachments( + createAttachments( versionedContent.getId(), contentVersionId, versionedContent.getAttachments() ) ); + } } private CreateAttachments createAttachments( final ContentId contentId, final ContentVersionId contentVersionId, diff --git a/modules/admin/admin-impl/src/test/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResourceTest.java b/modules/admin/admin-impl/src/test/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResourceTest.java index ca0fa335ae6..3f3168ea798 100644 --- a/modules/admin/admin-impl/src/test/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResourceTest.java +++ b/modules/admin/admin-impl/src/test/java/com/enonic/xp/admin/impl/rest/resource/content/ContentResourceTest.java @@ -2231,28 +2231,35 @@ public void testRevert_by_id() final ContentResource instance = getResourceInstance(); final RevertContentJson params = new RevertContentJson( "content-id", "versionKey" ); final Content updatedContent = createContent( "content-id", "content-name", "myapplication:content-type" ); + final PrincipalKey principalKey = RoleKeys.ADMIN; // mock final Content content = Mockito.mock( Content.class ); + final Content versionedContent = Mockito.mock( Content.class ); final ByteSource byteSource = Mockito.mock( ByteSource.class ); + final ContentVersion contentVersion = Mockito.mock( ContentVersion.class ); final Attachments attachments = Attachments.create().add( Attachment.create(). name( "attachment" ).mimeType( "mimeType" ).size( 1000L ).build() ).build(); - Mockito.when( content.getId() ).thenReturn( ContentId.from( "nodeId" ) ); - Mockito.when( content.getAttachments() ).thenReturn( attachments ); - Mockito.when( contentService.getByIdAndVersionId( any( ContentId.class ), any( ContentVersionId.class ) ) ).thenReturn( content ); + Mockito.when( versionedContent.getId() ).thenReturn( ContentId.from( "nodeId" ) ); + Mockito.when( versionedContent.getAttachments() ).thenReturn( attachments ); + Mockito.when( contentVersion.getModifier() ).thenReturn( principalKey ); + Mockito.when( contentVersion.getId() ).thenReturn( ContentVersionId.from( "contentVersionId" ) ); + Mockito.when( contentService.getByIdAndVersionId( any( ContentId.class ), any( ContentVersionId.class ) ) ).thenReturn( versionedContent ); + Mockito.when( contentService.getById( any( ContentId.class ) ) ).thenReturn( content ); Mockito.when( contentService.update( any( UpdateContentParams.class ) ) ).thenReturn( updatedContent ); Mockito.when( contentService.getBinary( any( ContentId.class ), any( ContentVersionId.class ), any( BinaryReference.class ) ) ).thenReturn( byteSource ); + Mockito.when( contentService.getActiveVersion( any( GetActiveContentVersionsParams.class ) ) ).thenReturn( contentVersion ); // test - final ContentJson result = instance.revert( params ); + final ContentVersionJson result = instance.revert( params ); // assert assertNotNull( result ); - assertEquals( "content-id", result.getId() ); + assertEquals( "contentVersionId", result.getId() ); // verify Mockito.verify( this.contentService, Mockito.times( 1 ) ). @@ -2261,6 +2268,10 @@ public void testRevert_by_id() getBinary( any( ContentId.class ), any( ContentVersionId.class ), any( BinaryReference.class ) ); Mockito.verify( this.contentService, Mockito.times( 1 ) ). update( any( UpdateContentParams.class ) ); + Mockito.verify( this.contentService, Mockito.times( 1 ) ). + getById( any( ContentId.class ) ); + Mockito.verify( this.contentService, Mockito.times( 1 ) ). + getActiveVersion( any( GetActiveContentVersionsParams.class ) ); Mockito.verifyNoMoreInteractions( contentService ); } @@ -2271,25 +2282,37 @@ public void testRevert_by_path() final ContentResource instance = getResourceInstance(); final RevertContentJson params = new RevertContentJson( "/content-name", "versionKey" ); final Content updatedContent = createContent( "content-id", "content-name", "myapplication:content-type" ); + final PrincipalKey principalKey = RoleKeys.ADMIN; // mock final Content content = Mockito.mock( Content.class ); - Mockito.when( contentService.getByPathAndVersionId( any( ContentPath.class ), any( ContentVersionId.class ) ) ).thenReturn( content ); + final Content versionedContent = Mockito.mock( Content.class ); + final ContentVersion contentVersion = Mockito.mock( ContentVersion.class ); + + Mockito.when( versionedContent.getId() ).thenReturn( ContentId.from( "nodeId" ) ); + Mockito.when( contentVersion.getModifier() ).thenReturn( principalKey ); + Mockito.when( contentVersion.getId() ).thenReturn( ContentVersionId.from( "contentVersionId" ) ); + Mockito.when( contentService.getByPathAndVersionId( any( ContentPath.class ), any( ContentVersionId.class ) ) ).thenReturn( versionedContent ); Mockito.when( contentService.update( any( UpdateContentParams.class ) ) ).thenReturn( updatedContent ); + Mockito.when( contentService.getById( any( ContentId.class ) ) ).thenReturn( content ); + Mockito.when( contentService.getActiveVersion( any( GetActiveContentVersionsParams.class ) ) ).thenReturn( contentVersion ); // test - final ContentJson result = instance.revert( params ); + final ContentVersionJson result = instance.revert( params ); // assert assertNotNull( result ); - assertEquals( "content-id", result.getId() ); - assertEquals( "/content-name", result.getPath() ); + assertEquals( "contentVersionId", result.getId() ); // verify Mockito.verify( this.contentService, Mockito.times( 1 ) ). getByPathAndVersionId( any( ContentPath.class ), any( ContentVersionId.class ) ); Mockito.verify( this.contentService, Mockito.times( 1 ) ). update( any( UpdateContentParams.class ) ); + Mockito.verify( this.contentService, Mockito.times( 1 ) ). + getById( any( ContentId.class ) ); + Mockito.verify( this.contentService, Mockito.times( 1 ) ). + getActiveVersion( any( GetActiveContentVersionsParams.class ) ); Mockito.verifyNoMoreInteractions( contentService ); } diff --git a/modules/core/core-api/src/main/java/com/enonic/xp/content/ContentService.java b/modules/core/core-api/src/main/java/com/enonic/xp/content/ContentService.java index df89c30798f..e298db6770a 100644 --- a/modules/core/core-api/src/main/java/com/enonic/xp/content/ContentService.java +++ b/modules/core/core-api/src/main/java/com/enonic/xp/content/ContentService.java @@ -101,6 +101,8 @@ public interface ContentService GetActiveContentVersionsResult getActiveVersions( GetActiveContentVersionsParams params ); + ContentVersion getActiveVersion(GetActiveContentVersionsParams params); + SetActiveContentVersionResult setActiveContentVersion( ContentId contentId, ContentVersionId versionId ); ByteSource getBinary( ContentId contentId, BinaryReference binaryReference ); diff --git a/modules/core/core-content/src/main/java/com/enonic/xp/core/impl/content/ContentServiceImpl.java b/modules/core/core-content/src/main/java/com/enonic/xp/core/impl/content/ContentServiceImpl.java index 206fe849e6b..88d8a8778bf 100644 --- a/modules/core/core-content/src/main/java/com/enonic/xp/core/impl/content/ContentServiceImpl.java +++ b/modules/core/core-content/src/main/java/com/enonic/xp/core/impl/content/ContentServiceImpl.java @@ -20,6 +20,7 @@ import com.google.common.util.concurrent.ThreadFactoryBuilder; import com.enonic.xp.app.ApplicationKey; +import com.enonic.xp.content.ActiveContentVersionEntry; import com.enonic.xp.content.ApplyContentPermissionsParams; import com.enonic.xp.content.ApplyContentPermissionsResult; import com.enonic.xp.content.CompareContentParams; @@ -28,6 +29,7 @@ import com.enonic.xp.content.CompareContentsParams; import com.enonic.xp.content.Content; import com.enonic.xp.content.ContentAccessException; +import com.enonic.xp.content.ContentConstants; import com.enonic.xp.content.ContentDependencies; import com.enonic.xp.content.ContentId; import com.enonic.xp.content.ContentIds; @@ -39,6 +41,7 @@ import com.enonic.xp.content.ContentService; import com.enonic.xp.content.ContentValidityParams; import com.enonic.xp.content.ContentValidityResult; +import com.enonic.xp.content.ContentVersion; import com.enonic.xp.content.ContentVersionId; import com.enonic.xp.content.Contents; import com.enonic.xp.content.CreateContentParams; @@ -913,6 +916,17 @@ public GetActiveContentVersionsResult getActiveVersions( final GetActiveContentV execute(); } + @Override + public ContentVersion getActiveVersion( final GetActiveContentVersionsParams params ) + { + final GetActiveContentVersionsResult activeVersions = getActiveVersions( params ); + + final ActiveContentVersionEntry activeVersion = activeVersions.getActiveContentVersions().stream().filter( + contentVersion -> ContentConstants.BRANCH_DRAFT.equals( contentVersion.getBranch() ) ).findFirst().orElse( null ); + + return activeVersion != null ? activeVersion.getContentVersion() : null; + } + @Override public SetActiveContentVersionResult setActiveContentVersion( final ContentId contentId, final ContentVersionId versionId ) {