Re: SDataSet.removeRecord(SRecordInstance rinst)

Hello,

I check our code, we aren't using SDataSet.attach / 
createDetachedInstance anymore.

But isDirty() on SRecordInstance is still used. Even if in some case we 
already know that the record is dirty because we get it from 
dataset.getDirtyRecord(), in other it helps.

I test our application with your updates and all is OK. The bug is still 
fixed !

I have more information about serialization. Actually, it was a non Sorm 
object witch used a SRecordMeta that causes problems. The reference to 
the meta is now transient
and there is no serialization error coming from sorm now.

Damien

[email protected] a écrit :
>
> Of course, cannot remove SRecordInstant.dirty because of deprecated 
> instances detached from DataSets. Reverted + comment!
>
> Can we get rid of this, or are you still using it?
>
> Have removed serializatbility of SRecord|FieldMeta, plus comment.
>
> Please update your version and confirm all is OK.
>
> I suspect your Tomcat problem is unrelated. If you make the 
> SRecordMeta variable a static per the normal patterns then it is hard 
> to see how you could be going wrong.
>
> Anthony
>
> At 01:28 PM 21/02/2009, you wrote:
> >Thanks for that.
> >
> >I cleaned it up a little. Removed SRecordInstant.dirty which is now 
> redundant.
> >
> >removeRecord is now just
> >
> > public void removeRecord(SRecordInstance rinst) {
> > rinst.setDirty(false);
> > records.remove(rinst);
> > rinst.destroy();
> > }
> >
> >Please check it out and confirm it works for you.
> >
> >The only reason to move removeRecord is because deleteRecord is on 
> SRecordInstance. No matter, leave it where it is. Better because clear 
> that it is data set oriented.
> >
> >Anthony
> >
> >
> >At 08:49 PM 20/02/2009, you wrote:
> >
> >>Hello,
> >>
> >>SDataSet.removeRecord now call instance.setDirty(false), and it 
> works fine.
> >>
> >>I don't really see why it would be interesting to move removeRecord to
> >>SRecordInstance.
> >>It would mean "The record removes himself from the dataSet" instead of
> >>"We ask the dataset to remove a record".
> >>
> >>Anyway I can commit the fix and let you check for where to put 
> removeRecord.
> >>
> >>Damien
> >>
> >><mailto:anthony%40berglas.org>[email protected] 
> <mailto:anthony%40berglas.org> a écrit :
> >>>
> >>> Hello Damien,
> >>>
> >>> The checks "if records obtained from SDataSet.getDirtyRecords are not
> >>> null" should already be there, but please check the checks. Note that
> >>> DataSet.removeFromDirtyList set it to null.
> >>>
> >>> removeRecord should call instance.setDirty(false) which will 
> remove it
> >>> from the cache and clear the updateListIndex (which should be
> >>> renamed). (Maybe removeRecord should be moved to
> >>> SRecordInstance.rawRemoveRecord, although all the other DataSet only
> >>> methods are on the DataSet. The comment should certainly refer to the
> >>> delete method.)
> >>>
> >>> Please check this in when done.
> >>>
> >>> Anthony
> >>>
> >>> At 11:54 PM 17/02/2009, you wrote:
> >>>
> >>> >I've just implemented the fix with setting null the removed record in
> >>> >dirty list.
> >>> >
> >>> >The only thing it implies is to check if records obtained from
> >>> >SDataSet.getDirtyRecords are not null before working with them.
> >>> >
> >>> >I also add a test in SDataSet.removeRecord to check consistence 
> between
> >>> >dirtyRecordList from SDataset and dirtyRecordsIndex from 
> SRecordInstance.
> >>> >
> >>> >It seems to work better when encountering this case !
> >>> >
> >>> >I answer about the serialization in the other thread.
> >>> >
> >>> 
> ><mailto:anthony%40berglas.org><mailto:anthony%40berglas.org>[email protected] 
> <mailto:anthony%40berglas.org>
> >>> <mailto:anthony%40berglas.org> a écrit :
> >>> >>
> >>> >> Hello Damien,
> >>> >>
> >>> >> Yes, I think that you have found a bug.
> >>> >>
> >>> >> It will only arise in practice if you actually call removeRecord.
> >>> >> Normal operations such as Flush and Delete do not remove the 
> records
> >>> >> from the dirty list, and so will not trigger the bug.
> >>> >>
> >>> >> For now please just null the entry in dirtyRecords. Do not try to
> >>> >> update the updateListIndex references. That is because Flush and
> >>> >> Delete just null the entry, and the behavior should be consistent.
> >>> >>
> >>> >> (If ever you did want to actually dirtyRecords.remove the entry 
> then
> >>> >> it should be done consistently everywhere. That is a 
> substantial job,
> >>> >> and I think that the benefit, if any, would be minimal.)
> >>> >>
> >>> >> Please also rename updateListIndex to dirtyRecordsIndex to be
> >>> >> consistent. (dirtyRecords used to be called updateList.)
> >>> >>
> >>> >> But first please also look at the exception that you received 
> when you
> >>> >> found the error. It should have been a clear, fail fast message 
> about
> >>> >> inconsistent data structures. If not, we may need another test. 
> Also
> >>> >> please add a failing test case before fixing the bug.
> >>> >>
> >>> >> Finally, please do try commenting out "implements Serializable" 
> from
> >>> >> SRecordMeta and SFieldMeta. If it does not break your 
> application then
> >>> >> we will remove them for good. I was always suspicious about 
> them, and
> >>> >> now I am pretty sure they are plain wrong.
> >>> >>
> >>> >> Thanks,
> >>> >>
> >>> >> Anthony
> >>> >>
> >>> >> At 08:27 PM 16/02/2009, you wrote:
> >>> >>
> >>> >> >Hello,
> >>> >> >
> >>> >> >I just find a bug in the method removeRecord from the SDataSet 
> object.
> >>> >> >It seems that each SRecordInstance in the dataset have a
> >>> updateListIndex
> >>> >> >attribute. This attribute is used to quickly find the index in 
> the
> >>> dirty
> >>> >> >record list from a dataset. When removeRecord is used, the dirty
> >>> record
> >>> >> >list shifts all elements from the deleted one to the end of the
> >>> list, so
> >>> >> >that the deleted record is removed ! All the record
> >>> updateListIndex from
> >>> >> >this point are no longer correct. They bind their old position 
> in the
> >>> >> >list, not the new.
> >>> >> >
> >>> >> >I don't know which way is better to avoid the bug, substract 1
> >>> from all
> >>> >> >index that have been shifted or not remove the record but set the
> >>> array
> >>> >> >entry to null ... ? Setting to null is quicker but may be the 
> cause of
> >>> >> >futur bugs. I could trust the unit test to validate the fix. I 
> think
> >>> >> >shifting the updateListIndex is more reliable even if it add 
> more cpu
> >>> >> work.
> >>> >> >
> >>> >> >--
> >>> >> >HOSTIN Damien - Equipe R&D
> >>> >> >Tel:+33(0)4 63 05 95 40
> >>> >> >Société Axège
> >>> >> >23 rue Saint Simon
> >>> >> >63000 Clermont Ferrand
> >>> >> >www.axege.com
> >>> >> >
> >>> >> >
> >>> >>
> >>> >> Dr Anthony Berglas,
> >>> 
> <mailto:anthony%40berglas.org><mailto:anthony%40berglas.org>[email protected] 
> <mailto:anthony%40berglas.org>
> >>> <mailto:anthony%40berglas.org> <mailto:anthony%40berglas.org>
> >>> >> Mobile: +61 4 4838 8874
> >>> >> Just because it is possible to push twigs along the ground with
> >>> ones nose
> >>> >> does not necessarily mean that is the best way to collect firewood.
> >>> >>
> >>> >>
> >>> >>
> >>> >> ----------------------------------------------------------
> >>> >>
> >>> >
> >>> >--
> >>> >HOSTIN Damien - Equipe R&D
> >>> >Tel:+33(0)4 63 05 95 40
> >>> >Société Axège
> >>> >23 rue Saint Simon
> >>> >63000 Clermont Ferrand
> >>> >www.axege.com
> >>> >
> >>> >
> >>>
> >>> Dr Anthony Berglas, 
> <mailto:anthony%40berglas.org>[email protected] 
> <mailto:anthony%40berglas.org> <mailto:anthony%40berglas.org>
> >>> Mobile: +61 4 4838 8874
> >>> Just because it is possible to push twigs along the ground with 
> ones nose
> >>> does not necessarily mean that is the best way to collect firewood.
> >>>
> >>>
> >>
> >>--
> >>HOSTIN Damien - Equipe R&D
> >>Tel:+33(0)4 63 05 95 40
> >>Société Axège
> >>23 rue Saint Simon
> >>63000 Clermont Ferrand
> >>www.axege.com
> >>
> >>
> >
> >Dr Anthony Berglas, [email protected] 
> <mailto:anthony%40berglas.org> Mobile: +61 4 4838 8874
> >Just because it is possible to push twigs along the ground with ones nose
> >does not necessarily mean that is the best way to collect firewood.
>
> Dr Anthony Berglas, [email protected] <mailto:anthony%40berglas.org> 
> Mobile: +61 4 4838 8874
> Just because it is possible to push twigs along the ground with ones nose
> does not necessarily mean that is the best way to collect firewood.
>
> 


-- 
HOSTIN Damien - Equipe R&D
Tel:+33(0)4 63 05 95 40
Société Axège
23 rue Saint Simon
63000 Clermont Ferrand
www.axege.com




------------------------------------

Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/SimpleORM/

<*> Your email settings:
    Individual Email | Traditional

<*> To change settings online go to:
    http://groups.yahoo.com/group/SimpleORM/join
    (Yahoo! ID required)

<*> To change settings via email:
    mailto:[email protected] 
    mailto:[email protected]

<*> To unsubscribe from this group, send an email to:
    [email protected]

<*> Your use of Yahoo! Groups is subject to:
    http://docs.yahoo.com/info/terms/