Skip to content

Commit

Permalink
Attachment modifications are not synced in layers #9604
Browse files Browse the repository at this point in the history
  • Loading branch information
@vbradnitski
vbradnitski committed Aug 5, 2022
1 parent d67cc12 commit 43d0f39
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 91 deletions.
5 changes: 3 additions & 2 deletions modules/core/core-api/src/main/java/com/enonic/xp/attachment/Attachment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,14 @@ public boolean equals( final Object o )
}
final Attachment that = (Attachment) o;
return Objects.equals( this.name, that.name ) && Objects.equals( this.mimeType, that.mimeType ) &&
Objects.equals( this.label, that.label ) && Objects.equals( this.size, that.size );
Objects.equals( this.label, that.label ) && Objects.equals( this.size, that.size ) &&
Objects.equals( this.textContent, that.textContent );
}

@Override
public int hashCode()
{
return Objects.hash( name, mimeType, label, size );
return Objects.hash( name, mimeType, label, size, textContent );
}

@Override
Expand Down
7 changes: 7 additions & 0 deletions modules/core/core-api/src/test/java/com/enonic/xp/attachment/AttachmentTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import org.junit.jupiter.api.Test;

import nl.jqno.equalsverifier.EqualsVerifier;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -101,4 +103,9 @@ public void compareAttachments()
assertFalse( a1.equals( a2Builder.size( 2048 ).build() ) );
}

@Test
void equalsContract()
{
EqualsVerifier.forClass( Attachment.class ).verify();
}
}
4 changes: 1 addition & 3 deletions ...core-content/src/main/java/com/enonic/xp/core/impl/content/ParentContentSynchronizer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import com.enonic.xp.context.Context;
import com.enonic.xp.context.ContextAccessor;
import com.enonic.xp.context.ContextBuilder;
import com.enonic.xp.media.MediaInfoService;
import com.enonic.xp.node.NodePath;
import com.enonic.xp.project.ProjectName;
import com.enonic.xp.security.PrincipalKey;
Expand All @@ -46,7 +45,7 @@ public final class ParentContentSynchronizer
private final ContentService contentService;

@Activate
public ParentContentSynchronizer( @Reference final ContentService contentService, @Reference final MediaInfoService mediaInfoService )
public ParentContentSynchronizer( @Reference final ContentService contentService )
{
this.contentService = contentService;

Expand All @@ -66,7 +65,6 @@ public ParentContentSynchronizer( @Reference final ContentService contentService
.build() );
syncCommandCreators.put( ContentSyncEventType.UPDATED, params -> UpdatedEventSyncCommand.create()
.contentService( contentService )
.mediaInfoService( mediaInfoService )
.params( params )
.build() );
syncCommandCreators.put( ContentSyncEventType.DELETED,
Expand Down
92 changes: 23 additions & 69 deletions ...e/core-content/src/main/java/com/enonic/xp/core/impl/content/UpdatedEventSyncCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,33 +5,22 @@
import com.google.common.base.Preconditions;
import com.google.common.io.ByteSource;

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.content.Content;
import com.enonic.xp.content.ContentIds;
import com.enonic.xp.content.ContentInheritType;
import com.enonic.xp.content.Media;
import com.enonic.xp.content.UpdateContentParams;
import com.enonic.xp.content.WorkflowState;
import com.enonic.xp.icon.Thumbnail;
import com.enonic.xp.media.MediaInfo;
import com.enonic.xp.media.MediaInfoService;
import com.enonic.xp.schema.content.ContentTypeFromMimeTypeResolver;
import com.enonic.xp.schema.content.ContentTypeName;
import com.enonic.xp.security.PrincipalKey;
import com.enonic.xp.util.BinaryReferences;

final class UpdatedEventSyncCommand
extends AbstractContentEventSyncCommand
{
private final MediaInfoService mediaInfoService;

UpdatedEventSyncCommand( final Builder builder )
{
super( builder );
this.mediaInfoService = builder.mediaInfoService;
}

public static Builder create()
Expand All @@ -56,8 +45,7 @@ private void doSync( final ContentToSync content )
{
final UpdateContentParams updateParams = updateParams( content.getSourceContent() );

doSyncMedia( content, updateParams );
doSyncThumbnail( content, updateParams );
doSyncAttachments( content, updateParams );

contentService.update( updateParams );
}
Expand All @@ -66,62 +54,36 @@ private void doSync( final ContentToSync content )
} );
}

private void doSyncMedia( final ContentToSync content, final UpdateContentParams updateParams )
private void doSyncAttachments( final ContentToSync content, final UpdateContentParams updateParams )
{
if ( content.getSourceContent() instanceof Media )
if ( !Objects.equals( content.getSourceContent().getAttachments(), content.getTargetContent().getAttachments() ) )
{
final Media sourceMedia = (Media) content.getSourceContent();

final Attachment mediaAttachment = sourceMedia.getMediaAttachment();

final ByteSource sourceBinary = content.getSourceContext()
.callWith( () -> contentService.getBinary( sourceMedia.getId(), mediaAttachment.getBinaryReference() ) );
final MediaInfo mediaInfo = content.getSourceContext().callWith( () -> mediaInfoService.parseMediaInfo( sourceBinary ) );
updateParams.clearAttachments( true );
final Attachments sourceAttachments = content.getSourceContent().getAttachments();

final ContentTypeName type = ContentTypeFromMimeTypeResolver.resolve( mediaAttachment.getMimeType() );

final CreateAttachment createAttachment = CreateAttachment.create()
.name( mediaAttachment.getName() )
.mimeType( mediaAttachment.getMimeType() )
.label( "source" )
.byteSource( sourceBinary )
.text( type != null && type.isTextualMedia() ? mediaInfo.getTextContent() : null )
.build();
if ( sourceAttachments != null )
{
final CreateAttachments.Builder createAttachments = CreateAttachments.create();

updateParams.clearAttachments( true ).createAttachments( CreateAttachments.from( createAttachment ) );
}
}
sourceAttachments.forEach( ( sourceAttachment ) -> {
final ByteSource sourceBinary = content.getSourceContext()
.callWith(
() -> contentService.getBinary( content.getSourceContent().getId(), sourceAttachment.getBinaryReference() ) );

private void doSyncThumbnail( final ContentToSync content, final UpdateContentParams updateParams )
{
if ( !Objects.equals( content.getSourceContent().getThumbnail(), content.getTargetContent().getThumbnail() ) )
{
final Thumbnail sourceThumbnail = content.getSourceContent().getThumbnail();
final CreateAttachment createAttachment = CreateAttachment.create()
.name( sourceAttachment.getName() )
.mimeType( sourceAttachment.getMimeType() )
.byteSource( sourceBinary )
.text( sourceAttachment.getTextContent() )
.label( sourceAttachment.getLabel() )
.build();

if ( sourceThumbnail != null )
{
final ByteSource sourceBinary = content.getSourceContext()
.callWith( () -> contentService.getBinary( content.getSourceContent().getId(), sourceThumbnail.getBinaryReference() ) );
createAttachments.add( createAttachment );

final CreateAttachment createThumbnail = CreateAttachment.create()
.name( AttachmentNames.THUMBNAIL )
.mimeType( sourceThumbnail.getMimeType() )
.byteSource( sourceBinary )
.build();

final CreateAttachments.Builder createAttachments = CreateAttachments.create().add( createThumbnail );
if ( updateParams.getCreateAttachments() != null )
{
createAttachments.add( updateParams.getCreateAttachments() );
}
} );

updateParams.createAttachments( createAttachments.build() );
}
else
{
final Thumbnail targetThumbnail = content.getTargetContent().getThumbnail();
updateParams.removeAttachments( BinaryReferences.from( targetThumbnail.getBinaryReference() ) );
}
}
}

Expand Down Expand Up @@ -150,6 +112,7 @@ private boolean needToUpdate( final Content sourceContent, final Content targetC
!Objects.equals( sourceContent.getPage(), targetContent.getPage() ) ||
!Objects.equals( sourceContent.getThumbnail(), targetContent.getThumbnail() ) ||
!Objects.equals( sourceContent.getProcessedReferences(), targetContent.getProcessedReferences() ) ||
!Objects.equals( sourceContent.getAttachments(), targetContent.getAttachments() ) ||
sourceContent.isValid() != targetContent.isValid();
}

Expand All @@ -176,18 +139,9 @@ private UpdateContentParams updateParams( final Content source )
public static class Builder
extends AbstractContentEventSyncCommand.Builder<Builder>
{
private MediaInfoService mediaInfoService;

public Builder mediaInfoService( final MediaInfoService mediaInfoService )
{
this.mediaInfoService = mediaInfoService;
return this;
}

void validate()
{
super.validate();
Preconditions.checkNotNull( mediaInfoService, "mediaInfoService must be set." );
Preconditions.checkArgument( params.getContents().stream().allMatch( content -> content.getSourceContent() != null ),
"sourceContent must be set." );
Preconditions.checkArgument( params.getContents().stream().allMatch( content -> content.getTargetContent() != null ),
Expand Down
80 changes: 66 additions & 14 deletions ...st/itest-core/src/test/java/com/enonic/xp/core/content/ParentContentSynchronizerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ protected void setUpNode()
{
super.setUpNode();

synchronizer = new ParentContentSynchronizer( this.contentService, this.mediaInfoService );
synchronizer = new ParentContentSynchronizer( this.contentService );

syncContentService =
new SyncContentServiceImpl( contentTypeService, nodeService, eventPublisher, pageDescriptorService, partDescriptorService,
Expand Down Expand Up @@ -309,10 +309,8 @@ public void updateDataChanged()
final Content sourceContent = sourceContext.callWith( () -> createContent( ContentPath.ROOT, "content1" ) );
final Content targetContent = syncCreated( sourceContent.getId() );

sourceContext.runWith( () -> {
contentService.update(
new UpdateContentParams().contentId( sourceContent.getId() ).editor( ( edit -> edit.data = new PropertyTree() ) ) );
} );
sourceContext.callWith( () -> contentService.update(
new UpdateContentParams().contentId( sourceContent.getId() ).editor( ( edit -> edit.data = new PropertyTree() ) ) ) );

final Content targetContentUpdated = syncUpdated( sourceContent.getId() );

Expand All @@ -327,17 +325,17 @@ public void updateMediaChanged()
final Content sourceContent = sourceContext.callWith( () -> createMedia( "media", ContentPath.ROOT ) );
final Content targetContent = syncCreated( sourceContent.getId() );

sourceContext.runWith( () -> {
sourceContext.callWith( () -> {
try
{
contentService.update( new UpdateMediaParams().content( sourceContent.getId() )
.name( "new name" )
.byteSource( loadImage( "darth-small.jpg" ) )
.mimeType( "image/jpeg" )
.artist( "artist" )
.copyright( "copy" )
.tags( "my new tags" )
.caption( "caption" ) );
return contentService.update( new UpdateMediaParams().content( sourceContent.getId() )
.name( "new name" )
.byteSource( loadImage( "darth-small.jpg" ) )
.mimeType( "image/jpeg" )
.artist( "artist" )
.copyright( "copy" )
.tags( "my new tags" )
.caption( "caption" ) );
}
catch ( IOException e )
{
Expand All @@ -363,6 +361,60 @@ public void updateMediaChanged()
assertEquals( "my new tags", targetContentUpdated.getData().getString( "tags" ) );
}


@Test
public void updateAttachmentsChanged()
throws Exception
{
final Content sourceContent = sourceContext.callWith( () -> createMedia( "media", ContentPath.ROOT ) );
syncCreated( sourceContent.getId() );

Content sourceContentUpdated = sourceContext.callWith( () -> {
try
{
return contentService.update( new UpdateContentParams().contentId( sourceContent.getId() )
.createAttachments( CreateAttachments.create()
.add( CreateAttachment.create()
.name( "new name" )
.byteSource( loadImage( "darth-small.jpg" ) )
.mimeType( "image/jpeg" )
.build() )
.build() ) );
}
catch ( IOException e )
{
throw new RuntimeException( e );
}
} );

Content targetContentUpdated = syncUpdated( sourceContent.getId() );

assertEquals( sourceContentUpdated.getAttachments(), targetContentUpdated.getAttachments() );

sourceContentUpdated = sourceContext.callWith( () -> {
try
{
return contentService.update( new UpdateContentParams().contentId( sourceContent.getId() )
.clearAttachments( true )
.createAttachments( CreateAttachments.create()
.add( CreateAttachment.create()
.name( "new name 1" )
.byteSource( loadImage( "darth-small.jpg" ) )
.mimeType( "image/jpeg" )
.build() )
.build() ) );
}
catch ( IOException e )
{
throw new RuntimeException( e );
}
} );

targetContentUpdated = syncUpdated( sourceContent.getId() );

assertEquals( sourceContentUpdated.getAttachments(), targetContentUpdated.getAttachments() );
}

@Test
public void updateThumbnailCreated()
throws Exception
Expand Down
2 changes: 1 addition & 1 deletion .../itest-core/src/test/java/com/enonic/xp/core/content/ProjectContentEventListenerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ protected void setUpNode()
{
super.setUpNode();

final ParentContentSynchronizer synchronizer = new ParentContentSynchronizer( contentService, mediaInfoService );
final ParentContentSynchronizer synchronizer = new ParentContentSynchronizer( contentService );
listener = new ProjectContentEventListener( this.projectService, synchronizer );

eventCaptor = ArgumentCaptor.forClass( Event.class );
Expand Down
2 changes: 1 addition & 1 deletion ...s/itest/itest-core/src/test/java/com/enonic/xp/core/content/ProjectEventListenerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ protected void setUpNode()
{
super.setUpNode();

final ParentContentSynchronizer synchronizer = new ParentContentSynchronizer( contentService, mediaInfoService );
final ParentContentSynchronizer synchronizer = new ParentContentSynchronizer( contentService );
listener = new ProjectEventListener( this.projectService, this.taskService, synchronizer );

eventCaptor = ArgumentCaptor.forClass( Event.class );
Expand Down
2 changes: 1 addition & 1 deletion ...itest/itest-core/src/test/java/com/enonic/xp/core/content/SyncContentServiceImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public void setUpNode()
{
super.setUpNode();

synchronizer = new ParentContentSynchronizer( contentService, mediaInfoService );
synchronizer = new ParentContentSynchronizer( contentService );

syncContentService =
new SyncContentServiceImpl( contentTypeService, nodeService, eventPublisher, pageDescriptorService, partDescriptorService,
Expand Down

0 comments on commit 43d0f39

Please sign in to comment.