SandstoneDB and a dictionary

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

SandstoneDB and a dictionary

David Zmick-2
Saving a dictionary that has references to other objects as keys results in data loss in the dictionary.

I have created a simplified example to explain my problem:

SDActiveRecord subclass: #Test
instanceVariableNames: 'as bs'
classVariableNames: ''
poolDictionaries: ''
category: 'Sandbox'

Test>>initialize
super initialize.
self as1: OrderedCollection new.
self bs: OrderedCollection new.
self as add: (A new title: 1).
self as add: (A new title: 2).
self as add: (A new title: 3).
self bs add: (B new title: 1).
self bs add: (B new title: 2).
self bs add: (B new title: 3).
self as do: [:a | self bs do: [:b | a dict at: b put: 1]].

Object subclass: #A
instanceVariableNames: 'title dict'
classVariableNames: ''
poolDictionaries: ''
category: 'Sandbox'

A>>initialize
self dict: Dictionary new.

Object subclass: #B
instanceVariableNames: 'title'
classVariableNames: ''
poolDictionaries: ''
category: 'Sandbox'

Evaluating Test new and inspecting the result behaves as expected, but after evaluating Test new save, verifying the Test findAll has the expected values (which it does) and closing the image without saving, opening the image and attempting to reload the objects results in each instance of A's dictionary looking like this.

dict: a Dictionary(nil->1)

it should have said this(and did before closing and opening the image again):

dict: a Dictionary(a B->1 a B->1 a B->1)

Hopefully I have explained this problem somewhat well, it is difficult for me to find any other way to explain what I am experiencing. Should I not be using the B objects as dictionary keys? Am I using the wrong dictionary? I am lost. Thanks for any help.
--
David Zmick
Reply | Threaded
Open this post in threaded view
|

Re: SandstoneDB and a dictionary

Ramon Leon-5
On 06/29/2011 02:30 PM, David Zmick wrote:
> Saving a dictionary that has references to other objects as keys results
> in data loss in the dictionary.

Do all the tests pass in your image?  The test..

testDictionaryKeys
        | dad |
        dad := self manClass testPerson save.
        kid save.
        mom children: (Dictionary with: dad -> kid).
        mom save.
        self flushAndReload.
        self
                assert: (mom refreshed children at: dad refreshed )
                equals: kid refreshed

Should have already covered this scenario to ensure other records work
as keys.  #flushAndReload is the same process that happens during image
startup.  Could you possibly express your scenario as a failing test
like above using the existing mock objects?  If you can show me a
failing test I'll get the problem resolved much quicker.

--
Ramon Leon
http://onsmalltalk.com

Reply | Threaded
Open this post in threaded view
|

Re: SandstoneDB and a dictionary

David Zmick-2
All the tests are passing except SDFileStoreTests>>#testBigSave and it appears to be because commitTime is not < 3 seconds.

Do you want me to create a test in the actual test classes for Sandstonedb using the mom dad kid objects or can I do something like this?

this creates the error I am experiencing when I ran it in the workspace:

| t |
t := Test new.
t as1: OrderedCollection new.
t bs: OrderedCollection new.
t as add: (A new title: 1).

t bs add: (B new title: 1).
t as do: [:a | t bs do: [:b | a dict at: b put: 1]].
t save.

Test coolDown; warmUp.

t inspect.

I don't really know how I would translate this into a test case, I am still new to programming so if you could give me some pointers I would very much appreciate them.

On Wed, Jun 29, 2011 at 4:39 PM, Ramon Leon <[hidden email]> wrote:
On 06/29/2011 02:30 PM, David Zmick wrote:
Saving a dictionary that has references to other objects as keys results
in data loss in the dictionary.

Do all the tests pass in your image?  The test..

testDictionaryKeys
       | dad |
       dad := self manClass testPerson save.
       kid save.
       mom children: (Dictionary with: dad -> kid).
       mom save.
       self flushAndReload.
       self
               assert: (mom refreshed children at: dad refreshed )
               equals: kid refreshed

Should have already covered this scenario to ensure other records work as keys.  #flushAndReload is the same process that happens during image startup.  Could you possibly express your scenario as a failing test like above using the existing mock objects?  If you can show me a failing test I'll get the problem resolved much quicker.

--
Ramon Leon
http://onsmalltalk.com




--
David Zmick
Reply | Threaded
Open this post in threaded view
|

Re: SandstoneDB and a dictionary

Ramon Leon-5
On 06/29/2011 02:55 PM, David Zmick wrote:
>
> Do you want me to create a test in the actual test classes for
> Sandstonedb using the mom dad kid objects or can I do something like this?

If you can use the existing mocks, i.e. the man woman child classes, and
make it fail in a workspace like you just did, that would be very
helpful.  Look at the existing testDictionaryKeys and build a similar
one in the test class and get it to fail.  Send me the failing test.  If
you can do it in a workspace, you can do it in a test, but I'd start
with the test and forget the workspace all together.  You may find a
bug, or you may find you were wrong.

If you don't reply with a failing test, I may or may not find some time
tonight to look into your examples, but if you need/want it fixed fast,
send me a failing test.

--
Ramon Leon
http://onsmalltalk.com

Reply | Threaded
Open this post in threaded view
|

Re: SandstoneDB and a dictionary

David Zmick-2
Okay. So I wrote a test case with similar behavior and it worked correctly, but the difference between the test case objects and my objects is that all of the test case objects are active records that get saved, not just the top level object as in my code.

So I guess there is no bug, I just do not understand what I am doing, which is what I was expecting to be the case anyway.

So I guess my question is, do I need to make every object I am saving into a subclass of SDActiveRecord and go through everything and save everything, or should I be able to have, in my example code, the class Test be the only SDActiveRecord subclass and only worry about saving instances of Test?


On Wed, Jun 29, 2011 at 6:17 PM, Ramon Leon <[hidden email]> wrote:
On 06/29/2011 02:55 PM, David Zmick wrote:

Do you want me to create a test in the actual test classes for
Sandstonedb using the mom dad kid objects or can I do something like this?

If you can use the existing mocks, i.e. the man woman child classes, and make it fail in a workspace like you just did, that would be very helpful.  Look at the existing testDictionaryKeys and build a similar one in the test class and get it to fail.  Send me the failing test.  If you can do it in a workspace, you can do it in a test, but I'd start with the test and forget the workspace all together.  You may find a bug, or you may find you were wrong.

If you don't reply with a failing test, I may or may not find some time tonight to look into your examples, but if you need/want it fixed fast, send me a failing test.


--
Ramon Leon
http://onsmalltalk.com




--
David Zmick
Reply | Threaded
Open this post in threaded view
|

Re: SandstoneDB and a dictionary

Ramon Leon-5
On 06/29/2011 05:40 PM, David Zmick wrote:
> Okay. So I wrote a test case with similar behavior and it worked
> correctly, but the difference between the test case objects and my
> objects is that all of the test case objects are active records that get
> saved, not just the top level object as in my code.

Nope, that wasn't the difference that mattered.  You did find a bug, an
interesting one, but your test code was overly complex hiding what was
really going on.  Here's a simpler test case that exposes the bug more
directly.

testObjectFoundMultipleTimesInAggregate
        | obj key |
        obj := SDPersonMock new.
        obj children: Dictionary new.
        key := Object new.

         "store ref to key in two places"
        obj children at: key put: 1.
        obj children at: 1 put: key.

        self deny: (obj children keys includes: nil).
        self deny: (obj children values includes: nil).
        obj save.
        self flushAndReload.
        self deny: (obj children keys includes: nil).
        self deny: (obj children values includes: nil)

The bug has been fixed, pull down the latest code from SqueakSource and
your code with work fine.  As you can see from the test case above, the
issue was related to an object being references in multiple places
inside an active record.  During serialization, this exposed a silly bug
in the deep copy that I think I introduced recently with a round of
refactoring.  Thanks for the bug report.

--
Ramon Leon
http://onsmalltalk.com

Reply | Threaded
Open this post in threaded view
|

Re: SandstoneDB and a dictionary

David Zmick-2
Awesome!! Thank you so much for fixing it so quickly and being tolerant of my lack of knowledge. I have to say I am once again impressed by the smalltalk community. Thank you.

On Wed, Jun 29, 2011 at 11:11 PM, Ramon Leon <[hidden email]> wrote:
On 06/29/2011 05:40 PM, David Zmick wrote:
Okay. So I wrote a test case with similar behavior and it worked
correctly, but the difference between the test case objects and my
objects is that all of the test case objects are active records that get
saved, not just the top level object as in my code.

Nope, that wasn't the difference that mattered.  You did find a bug, an interesting one, but your test code was overly complex hiding what was really going on.  Here's a simpler test case that exposes the bug more directly.

testObjectFoundMultipleTimesInAggregate
       | obj key |
       obj := SDPersonMock new.
       obj children: Dictionary new.
       key := Object new.

       "store ref to key in two places"
       obj children at: key put: 1.
       obj children at: 1 put: key.

       self deny: (obj children keys includes: nil).
       self deny: (obj children values includes: nil).
       obj save.
       self flushAndReload.
       self deny: (obj children keys includes: nil).
       self deny: (obj children values includes: nil)

The bug has been fixed, pull down the latest code from SqueakSource and your code with work fine.  As you can see from the test case above, the issue was related to an object being references in multiple places inside an active record.  During serialization, this exposed a silly bug in the deep copy that I think I introduced recently with a round of refactoring.  Thanks for the bug report.


--
Ramon Leon
http://onsmalltalk.com




--
David Zmick