+1 Thanks, Marius Denis Gervalle wrote:

In XWiki(Hibernate|Jcr)Store#saveXWikiDoc(), the attachement list is persisted, but the attachement contents are not persisted. A) I would like to take care of also persisting attachment contents when these contents are loaded and marked dirty (XWikiAttachmentContent#isContentDirty() == true).

Why I would like to change this ?

1) I see no use case that would have to persist reference to an attachment and drop its content at the same time, which create records with a size field set to n bytes, while the attachment is 0 or m bytes. 2) When for example an XWikiDocument#copy() is followed by an XWiki#saveDocument() (on the newly created document), without also calling XWikiDocument#saveAllAttachements(), the cache store contains the documents with all their attachments contents, while the DB has not really persisted these contents. 3) In XWikiHibernateAttachmentStore#saveAttachmentContent(), the default is to persist the document, this is not consistent with the document not doing the same in reverse. (I would have better understand that persisting attachment contents does not persist the document.) 4) isContentDirty is properly maintained, and currently it is nearly unused, making the false impression that attachments content are persisted properly 5) This issue is invisible in normal XWiki usage, since we take care of saving attachments contents to avoid it everywhere, for example in XWiki#copyDocument(), attachments contents are saved "manually". 6) As a application developer, using the internal API in groovy, my collegue has missed it, and we have loose production data, since during testing, the cache has always hide the issue. I really dislike API that could introduce inconsistencies in the DB and moreover when you loose large amount of data.

B) This change has no real impact on any code that would take care of saving the attachments separatly, it will just be saved twice, isContentDirty() currently only prevent useless archiving. This could be optimized by not saving contents as well when it is not dirty, but I do not want a negative vote on the rest for just this. So please vote this as B)

C) I also would like to optimize existing internal usage, removing useless call to saveAttachmentContent() in XWiki.copyDocument() for example. This could have an impact on a custom store that does not take care of saving attachments contents while saving documents. I doubt many user has such a store and release notes could mention this incompatibility.

This is my +1 for A) B) and C)

Denis

+1 Caleb Denis Gervalle wrote:

In XWiki(Hibernate|Jcr)Store#saveXWikiDoc(), the attachement list is persisted, but the attachement contents are not persisted. A) I would like to take care of also persisting attachment contents when these contents are loaded and marked dirty (XWikiAttachmentContent#isContentDirty() == true).

Why I would like to change this ?

1) I see no use case that would have to persist reference to an attachment and drop its content at the same time, which create records with a size field set to n bytes, while the attachment is 0 or m bytes. 2) When for example an XWikiDocument#copy() is followed by an XWiki#saveDocument() (on the newly created document), without also calling XWikiDocument#saveAllAttachements(), the cache store contains the documents with all their attachments contents, while the DB has not really persisted these contents. 3) In XWikiHibernateAttachmentStore#saveAttachmentContent(), the default is to persist the document, this is not consistent with the document not doing the same in reverse. (I would have better understand that persisting attachment contents does not persist the document.) 4) isContentDirty is properly maintained, and currently it is nearly unused, making the false impression that attachments content are persisted properly 5) This issue is invisible in normal XWiki usage, since we take care of saving attachments contents to avoid it everywhere, for example in XWiki#copyDocument(), attachments contents are saved "manually". 6) As a application developer, using the internal API in groovy, my collegue has missed it, and we have loose production data, since during testing, the cache has always hide the issue. I really dislike API that could introduce inconsistencies in the DB and moreover when you loose large amount of data.

B) This change has no real impact on any code that would take care of saving the attachments separatly, it will just be saved twice, isContentDirty() currently only prevent useless archiving. This could be optimized by not saving contents as well when it is not dirty, but I do not want a negative vote on the rest for just this. So please vote this as B)

C) I also would like to optimize existing internal usage, removing useless call to saveAttachmentContent() in XWiki.copyDocument() for example. This could have an impact on a custom store that does not take care of saving attachments contents while saving documents. I doubt many user has such a store and release notes could mention this incompatibility.

This is my +1 for A) B) and C)

Denis

On 04/02/2010 10:08 AM, Denis Gervalle wrote:

In XWiki(Hibernate|Jcr)Store#saveXWikiDoc(), the attachement list is persisted, but the attachement contents are not persisted. A) I would like to take care of also persisting attachment contents when these contents are loaded and marked dirty (XWikiAttachmentContent#isContentDirty() == true).

Why I would like to change this ?

1) I see no use case that would have to persist reference to an attachment and drop its content at the same time, which create records with a size field set to n bytes, while the attachment is 0 or m bytes. 2) When for example an XWikiDocument#copy() is followed by an XWiki#saveDocument() (on the newly created document), without also calling XWikiDocument#saveAllAttachements(), the cache store contains the documents with all their attachments contents, while the DB has not really persisted these contents. 3) In XWikiHibernateAttachmentStore#saveAttachmentContent(), the default is to persist the document, this is not consistent with the document not doing the same in reverse. (I would have better understand that persisting attachment contents does not persist the document.)

I don't like this either. I think there's an issue I created some time ago, but Jira is down right now so I can't find it.

4) isContentDirty is properly maintained, and currently it is nearly unused, making the false impression that attachments content are persisted properly 5) This issue is invisible in normal XWiki usage, since we take care of saving attachments contents to avoid it everywhere, for example in XWiki#copyDocument(), attachments contents are saved "manually". 6) As a application developer, using the internal API in groovy, my collegue has missed it, and we have loose production data, since during testing, the cache has always hide the issue. I really dislike API that could introduce inconsistencies in the DB and moreover when you loose large amount of data.

+1

B) This change has no real impact on any code that would take care of saving the attachments separatly, it will just be saved twice, isContentDirty() currently only prevent useless archiving. This could be optimized by not saving contents as well when it is not dirty, but I do not want a negative vote on the rest for just this. So please vote this as B)

I'm not sure that this is really a problem. As far as I've seen, all code that manually saves attachments also resets isContentDirty to false, so (most) manually saved attachments won't get saved again,

C) I also would like to optimize existing internal usage, removing useless call to saveAttachmentContent() in XWiki.copyDocument() for example. This could have an impact on a custom store that does not take care of saving attachments contents while saving documents. I doubt many user has such a store and release notes could mention this incompatibility.

+1

This is my +1 for A) B) and C)

Denis

-- Sergiu Dumitriu http://purl.org/net/sergiu/