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/