heynemann / motorengine

Motorengine is a port of MongoEngine for Tornado.
http://motorengine.readthedocs.org
204 stars 67 forks source link

A ReferenceFiled with ListField #65

Closed antmanler closed 10 years ago

antmanler commented 10 years ago

See: document.py#L209

for value in document._values.get(field_name):
    if value:
        self.find_references(document=value, results=results)

value here is ObjectId not a valid document type, so there is not _fields

heynemann commented 10 years ago

Do you think you can come up with a test for this scenario? It would be easier to fix it that way...

antmanler commented 10 years ago

for example:

connect(db="test")

class Trade(Document):
    __collection__ = "Trade"
    created_time = DateTimeField(auto_now_on_insert=True)

class Order(Document):
    __collection__ = "Order"
    trade_history = ListField(ReferenceField(Trade))

@coroutine
def test():
    try:
        trade1 = yield Trade.objects.create()
        trade2 = yield Trade.objects.create()
        # create
        order = yield Order.objects.create(trade_history=[trade1, trade2])
        # get again
        order = yield Order.objects.get(id=order._id)
        # populate
        res = yield order.load_references(fields=["trade_history"])
        print res
    except Exception as e:
        print e

ioloop = tornado.ioloop.IOLoop.instance()
ioloop.add_callback(test)
ioloop.start()

this will raise

'ObjectId' object has no attribute '_fields'

heynemann commented 10 years ago

Thanks! I'll fix it asap!

partyzan commented 10 years ago

Was this issue fixed ? I get exactly the same error on the same scenario of ListField(ReferenceField)).

partyzan commented 10 years ago

I am using the following test:

    @gen_test
    def test_lis_ref(self):
        class Ref(Document):
            val = StringField()

        class Base(Document):
            list_val = ListField(ReferenceField(reference_document_type=Ref))

        ref1 = yield Ref.objects.create(val="v1")
        ref2 = yield Ref.objects.create(val="v2")
        ref3 = yield Ref.objects.create(val="v3")

        base = yield Base.objects.create(list_val=[ref1, ref2, ref3])

        base = yield Base.objects.get(base._id)
        self.assertIsNotNone(base)
        base.load_references()
        self.assertEqual(len(base.list_val), 3)
        self.assertEqual(base.list_val[0], ref1)

And I get the following error:

Error
Traceback (most recent call last):
  File "/Users/dimap/venv/vader/lib/python3.4/site-packages/tornado/testing.py", line 427, in wrapper
    functools.partial(f, self), timeout=timeout)
  File "/Users/dimap/venv/vader/lib/python3.4/site-packages/tornado/ioloop.py", line 389, in run_sync
    return future_cell[0].result()
  File "/Users/dimap/venv/vader/lib/python3.4/site-packages/tornado/concurrent.py", line 129, in result
    raise_exc_info(self.__exc_info)
  File "<string>", line 3, in raise_exc_info
  File "/Users/dimap/venv/vader/lib/python3.4/site-packages/tornado/stack_context.py", line 302, in wrapped
    ret = fn(*args, **kwargs)
  File "/Users/dimap/venv/vader/lib/python3.4/site-packages/tornado/gen.py", line 574, in inner
    self.set_result(key, result)
  File "/Users/dimap/venv/vader/lib/python3.4/site-packages/tornado/gen.py", line 500, in set_result
    self.run()
  File "/Users/dimap/venv/vader/lib/python3.4/site-packages/tornado/gen.py", line 531, in run
    yielded = self.gen.send(next)
  File "/Users/dimap/workspace/PycharmProjects/Transmit/tests/services/demo/test_dal.py", line 154, in test_lis_ref
    base.load_references()
  File "/Users/dimap/venv/vader/lib/python3.4/site-packages/tornado/concurrent.py", line 234, in wrapper
    raise_exc_info(exc_info)
  File "<string>", line 3, in raise_exc_info
  File "/Users/dimap/venv/vader/lib/python3.4/site-packages/tornado/concurrent.py", line 222, in wrapper
    result = f(*args, **kwargs)
  File "/Users/dimap/venv/vader/lib/python3.4/site-packages/motorengine/document.py", line 154, in load_references
    references = self.find_references(document=self, fields=fields)
  File "/Users/dimap/venv/vader/lib/python3.4/site-packages/motorengine/document.py", line 191, in find_references
    self.find_list_field(document, results, field_name, field)
  File "/Users/dimap/venv/vader/lib/python3.4/site-packages/motorengine/document.py", line 212, in find_list_field
    self.find_references(document=value, results=results)
  File "/Users/dimap/venv/vader/lib/python3.4/site-packages/motorengine/document.py", line 187, in find_references
    fields = [field for field in list(document._fields.items())]
AttributeError: 'ObjectId' object has no attribute '_fields'
heynemann commented 10 years ago

There was actually a bug with MotorEngine code, but your test is also missing a yield base.load_references. As with any other method in motorengine, load_references is async, and must be called with either a callback or a yield. I'm submitting the fix right now.

partyzan commented 10 years ago

Got it, thanks!

heynemann commented 10 years ago

@partyzan can you try installing motorengine master branch and verifying if the bug is gone?

$ pip install https://github.com/heynemann/motorengine/archive/master.zip

Then afterwards I'll release a new version.

Thanks!

partyzan commented 10 years ago

Done, it's working now. Used the following test:

    @gen_test
    def test_list_ref(self):
        class Ref(Document):
            val = StringField()

        class Base(Document):
            list_val = ListField(ReferenceField(reference_document_type=Ref))

        ref1 = yield Ref.objects.create(val="v1")
        ref2 = yield Ref.objects.create(val="v2")
        ref3 = yield Ref.objects.create(val="v3")

        base = yield Base.objects.create(list_val=[ref1, ref2, ref3])

        base = yield Base.objects.get(base._id)
        self.assertIsNotNone(base)
        yield base.load_references()
        self.assertEqual(len(base.list_val), 3)
        self.assertEqual(base.list_val[0].val, ref1.val)
heynemann commented 10 years ago

Btw @antmanler sorry for the long time it took to fix this issue...

heynemann commented 10 years ago

@partyzan you can also specify lazy=False in Base if you want those items to always be eager-loaded by motorengine.

I'm releasing a new version as soon as build is green.

partyzan commented 10 years ago

Awesome, thanks. --  Best Regards Dima Polsky

On July 6, 2014 at 5:58:22 PM, Bernardo Heynemann (notifications@github.com) wrote:

@partyzan you can also specify lazy=False in Base if you want those items to always be eager-loaded by motorengine.

I'm releasing a new version as soon as build is green.

— Reply to this email directly or view it on GitHub.

heynemann commented 10 years ago

Released under 0.8.8.