Re: SDataSet.removeRecord(SRecordInstance rinst)

damien hostin <[email protected]> Wed, 25 Feb 2009 17:41:15 +0100
Newsgroups gmane.comp.java.orm.simpleorm
Message-ID <[email protected]>
It seems to work... !

Damien

[email protected] a écrit :
>
> Good to hear that your problems are resolved.
>
> I have removed the deprecated attach(record) and the associated .dirty 
> flag. They could potentially be useful in some situation, particularly 
> if one wanted to move a record from one dataset to another (can not 
> see a use case for that). But they could also cause grief with bugs in 
> obscure corners. I prefer to keep it clean and simple.
>
> Please update and check it all still works for you.
>
> I am glad to get that resolved, it had been irritating me for some 
> time. And the Serialization is also now clean.
>
> Anthony
>
> At 11:37 PM 23/02/2009, you wrote:
>
> >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
> >
> ><mailto:anthony%40berglas.org>[email protected] 
> <mailto:anthony%40berglas.org> 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><mailto:anthony%40berglas.org>[email protected] 
> <mailto:anthony%40berglas.org>
> >> <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><mailto:anthony%40berglas.org>[email protected] 
> <mailto:anthony%40berglas.org>
> >> <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><mailto:anthony%40berglas.org>[email protected] 
> <mailto:anthony%40berglas.org>
> >> <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><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.
> >>
> >> 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.
>
> 


-- 
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/