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/