Skip to content

Commit

Permalink
XWIKI-15805: Error when using compare feature in history when an atta…
Browse files Browse the repository at this point in the history 
…chment is deleted

  * Looks for the XWikiAttachment in the recycle bin when performing an
attachment diff if the content is unavailable.
  * If the XWikiAttachment content is definitely unavailable (deleted
from the recycle bin too), then display an information for the users.
  • Loading branch information
@surli
surli committed Apr 18, 2019
1 parent 647f0b9 commit 5110849
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 14 deletions.
10 changes: 7 additions & 3 deletions xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/api/Attachment.java
View file
Expand Up @@ -155,11 +155,15 @@ public byte[] getContent() throws XWikiException
public byte[] getContentAsBytes() throws XWikiException
{
try {
return IOUtils.toByteArray(this.attachment.getContentInputStream(getXWikiContext()));
// the input stream can be null if the attachment has been deleted for example.
InputStream contentInputStream = this.attachment.getContentInputStream(getXWikiContext());
if (contentInputStream != null) {
return IOUtils.toByteArray(contentInputStream);
}
} catch (IOException ex) {
// This really shouldn't happen, but it's not nice to throw exceptions from scriptable APIs
return new byte[0];
// This really shouldn't happen.
}
return new byte[0];
}

public InputStream getContentInputStream()
Expand Down
22 changes: 11 additions & 11 deletions ...platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/doc/XWikiAttachment.java
View file
Expand Up @@ -901,9 +901,6 @@ private void reloadAttachmentContent(XWikiContext xcontext) throws XWikiExceptio
archivedVersion != null ? archivedVersion.getAttachment_content() : null;
if (archivedContent != null) {
setAttachment_content(archivedContent);
} else {
// Fall back on the version of the content stored in the xwikiattachment_content table.
loadAttachmentContent(xcontext);
}
}
}
Expand Down Expand Up @@ -1300,17 +1297,20 @@ public boolean equalsData(XWikiAttachment otherAttachment, XWikiContext xcontext
try {
if (getContentLongSize(xcontext) == otherAttachment.getContentLongSize(xcontext)) {
InputStream is = getContentInputStream(xcontext);

try {
InputStream otherIS = otherAttachment.getContentInputStream(xcontext);

if (is != null) {
try {
return IOUtils.contentEquals(is, otherIS);
InputStream otherIS = otherAttachment.getContentInputStream(xcontext);

if (otherIS != null) {
try {
return IOUtils.contentEquals(is, otherIS);
} finally {
otherIS.close();
}
}
} finally {
otherIS.close();
is.close();
}
} finally {
is.close();
}
}
} catch (Exception e) {
Expand Down
51 changes: 51 additions & 0 deletions ...i-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/doc/XWikiDocument.java
View file
Expand Up @@ -184,6 +184,7 @@
import com.xpn.xwiki.objects.classes.PropertyClass;
import com.xpn.xwiki.objects.classes.StaticListClass;
import com.xpn.xwiki.objects.classes.TextAreaClass;
import com.xpn.xwiki.store.AttachmentRecycleBinStore;
import com.xpn.xwiki.store.XWikiAttachmentStoreInterface;
import com.xpn.xwiki.store.XWikiHibernateAttachmentStore;
import com.xpn.xwiki.store.XWikiStoreInterface;
Expand Down Expand Up @@ -6651,9 +6652,11 @@ public List<AttachmentDiff> getAttachmentDiff(XWikiDocument fromDoc, XWikiDocume
for (XWikiAttachment origAttach : fromDoc.getAttachmentList()) {
String fileName = origAttach.getFilename();
XWikiAttachment newAttach = toDoc.getAttachment(fileName);
origAttach = retrieveDeletedAttachment(fromDoc, origAttach, context);
if (newAttach == null) {
difflist.add(new AttachmentDiff(fileName, org.xwiki.diff.Delta.Type.DELETE, origAttach, newAttach));
} else {
newAttach = retrieveDeletedAttachment(toDoc, newAttach, context);
try {
if (!origAttach.equalsData(newAttach, context)) {
difflist
Expand All @@ -6669,6 +6672,7 @@ public List<AttachmentDiff> getAttachmentDiff(XWikiDocument fromDoc, XWikiDocume
for (XWikiAttachment newAttach : toDoc.getAttachmentList()) {
String fileName = newAttach.getFilename();
XWikiAttachment origAttach = fromDoc.getAttachment(fileName);
newAttach = retrieveDeletedAttachment(toDoc, newAttach, context);
if (origAttach == null) {
difflist.add(new AttachmentDiff(fileName, org.xwiki.diff.Delta.Type.INSERT, origAttach, newAttach));
}
Expand All @@ -6677,6 +6681,53 @@ public List<AttachmentDiff> getAttachmentDiff(XWikiDocument fromDoc, XWikiDocume
return difflist;
}

private XWikiAttachment retrieveDeletedAttachment(XWikiDocument doc, XWikiAttachment attachment,
XWikiContext context)
{
XWikiAttachment result = null;

InputStream is = null;
try {
is = attachment.getContentInputStream(context);
if (is == null) {
AttachmentRecycleBinStore attachmentRecycleBinStore = context.getWiki().getAttachmentRecycleBinStore();
List<DeletedAttachment> allDeletedAttachments =
attachmentRecycleBinStore.getAllDeletedAttachments(this, context, true);

This comment has been minimized.

Sign in to view

Copy link
@mflorea

mflorea Jun 21, 2021

Member

You shouldn't use this here but the provided doc parameter because this method is used when computing the diff between documents and both documents are passed as parameters. The current document is just used to call the API because both documents can be null, but otherwise it shouldn't influence the diff (it shouldn't matter what the current document is).

for (DeletedAttachment deletedAttachment : allDeletedAttachments) {
XWikiAttachment restoredAttachment = deletedAttachment.restoreAttachment();
if (restoredAttachment.getDate().before(attachment.getDate())) {
break;
}
result = restoredAttachment;
}

if (result != null) {
if (!Objects.equals(attachment.getVersion(), result.getVersion())) {
result = result.getAttachmentRevision(attachment.getVersion(), context);
}
}
}
} catch (XWikiException e) {
LOGGER.error("Error while trying to load deleted attachment [{}] for doc [{}]", attachment, doc, e);
} finally {
if (is != null) {
try {
is.close();
} catch (IOException ex) {

}
}
}

if (result == null) {
result = attachment;
} else {
result.setDoc(doc);
}
return result;
}

/**
* Rename the current document and all the backlinks leading to it. Will also change parent field in all documents
* which list the document we are renaming as their parent.
Expand Down
1 change: 1 addition & 0 deletions ...i-platform-core/xwiki-platform-oldcore/src/main/resources/ApplicationResources.properties
View file
Expand Up @@ -1584,6 +1584,7 @@ web.history.changes.attachment.size=Size
web.history.changes.attachment.content=Content
web.history.changes.attachment.noContentChanges=Either this is not a text file or the file is too large
web.history.changes.privateInformation=Private information
web.history.changes.attachment.notAvailable=The content diff is not avaible. One attachment might have been deleted from the recycle bin.

core.viewers.diff.title=Changes for page <a href="{1}">{0}</a>
core.viewers.diff.from=From version {0}
Expand Down
3 changes: 3 additions & 0 deletions xwiki-platform-core/xwiki-platform-web/src/main/webapp/templates/diff_macros.vm
View file
Expand Up @@ -162,6 +162,9 @@
## Force the display of the diff header.
#set ($newContent = '')
#end
#if ($oldContent.length() == 0 || $newContent.length() == 0)
#set ($obfuscate = 'web.history.changes.attachment.notAvailable')
#end
#set ($contentLabel = $services.localization.render('web.history.changes.attachment.content'))
#maybeDisplayPropertyDiff($contentLabel $oldContent $newContent $obfuscate)
</dl>
Expand Down

0 comments on commit 5110849

Please sign in to comment.