Bug 114848 - ContentItem#beforeSave method does not always set the content section
ContentItem#beforeSave method does not always set the content section
Product: Red Hat Enterprise CMS
Classification: Retired
Component: domain (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: ccm-bugs-list
Jon Orris
Depends On:
Blocks: 113496
  Show dependency treegraph
Reported: 2004-02-03 10:41 EST by Daniel Berrange
Modified: 2007-04-18 13:02 EDT (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2004-02-11 17:39:26 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description Daniel Berrange 2004-02-03 10:41:37 EST
Description of problem:
During the APLAWS bulk content import, the 'section_id' field (aka the
Item <-> COntnetSection association) is never set on any of the
folders. This is despite the presence of the following code in

            if (getContentSection() == null) {

Now, this code is in turn wrapped in a isNew() check, so it is only
run once when the object is first created. Copious log4j debugging,
however, indicates that its not being run *EVER*.

Now, the first thing in the beforeSave method is a call 

        if (get(ANCESTORS) == null) {

Since the ANCESTORS denormalaization is written in Java, I suspected
this might be triggering a save, thus reseting the isNew flag. So I
added some more log4j debugging either side of this method. The
beforeSave method now looks something like:

        s_log.debug(m_bpad + "Start Before save " + this + " " +
m_wasNew + " " + isNew());
        m_wasNew = isNew();

        if (get(ANCESTORS) == null) {
        s_log.debug(m_bpad + "Next Before save " + this + " " +
m_wasNew +  " " + isNew());

        if (isNew()) {
            // Set the default content type.
            s_log.debug("Before save one-time birth stuff");

            // Set the default content section.
            if (getContentSection() == null) {
        s_log.debug(m_bpad + "End Before save " + this + " " +
m_wasNew + " " + isNew());
        m_bpad = m_bpad.substring(2);

NB, the 'm_bpad' variable I was using to see if beforeSave was in fact
re-entrant. After this I ran my program again and got the following logs:

--Start Before save [com.arsdigita.cms.Folder:{id=46003}]NM true true
--Start After save [com.arsdigita.cms.Folder:{id=64}] false false
--End After save [com.arsdigita.cms.Folder:{id=64}] false false
--Next Before save [com.arsdigita.cms.Folder:{id=46003}]M true false
--End Before save [com.arsdigita.cms.Folder:{id=46003}]M true false
--Start After save [com.arsdigita.cms.Folder:{id=46003}]M true false
--End After save [com.arsdigita.cms.Folder:{id=46003}] true false

NB, 64 is the ROOT folder & does have a content section set. 46003 is
the new folder I'm creating.

So, this shows that the call to setAncestors is indeed clearing the
'isNew' flag before the end of the beforeSave() method.

Code which deals with isNew() and save observers has hisotrically been
rather error prone, that it may be worth considering a pair of
beforeFirstSave() and afterFirstSave() observers to elminiate the need
for application programmers to hand-craft these semantics.

Version-Release number of selected component (if applicable):

How reproducible:

Steps to Reproduce:
Actual results:

Expected results:

Additional info:
Comment 1 Richard Li 2004-02-05 11:21:29 EST
ashah fixed @40019. the general problem of isNew has been noted
numerous times in many tickets; we want to fix it but that will take a
bit more time.
Comment 2 Jon Orris 2004-02-10 17:36:25 EST
Dan, can you confirm that Archit's fix corrects your specific problem?
Comment 3 Daniel Berrange 2004-02-11 10:53:00 EST
Yes it works fine now.

Note You need to log in before you can comment on or make changes to this bug.