Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: ListField may lost value if do iterate before some edit operation #632

Closed
RyanKung opened this issue Apr 25, 2014 · 3 comments
Closed

Comments

@RyanKung
Copy link

In this sample, I append data to list, saved, then do the 'for' loop, but when I call remove(), the append data was missed, and the 'remove' func didn't work.

# -*- coding: utf-8 -*-

import unittest
from mongoengine import *

class A(Document):
    list_b = ListField(ReferenceField('B'))

    def add_b_to_a(self, b):
        self.list_b.append(b)
        self.save()

class B(Document):
    pass


class DBTest(unittest.TestCase):

    def setUp(self):
        connect('test_mongoengine')

    def tearDown(self):
        A.drop_collection()
        B.drop_collection()

    def test_mongoengine_with_iterate(self):
        a = A().save()
        b = B().save()
        b_1 = B().save()

        a.list_b.append(b)
        a.save()
        a.add_b_to_a(b_1)

        for b in a.list_b: pass

        a.list_b.remove(b)

        self.assertTrue(b_1 in a.list_b)
        self.assertFalse(b in a.list_b)

    def test_mongoengine_without_iterate(self):
        a = A().save()
        b = B().save()
        b_1 = B().save()

        a.list_b.append(b)
        a.save()
        a.add_b_to_a(b_1)

        a.list_b.remove(b)

        self.assertTrue(b_1 in a.list_b)
        self.assertFalse(b in a.list_b)
F.
======================================================================
FAIL: test_mongoengine_with_iterate (test.DBTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "XXXX/dev/test.py", line 39, in test_mongoengine_with_iterate
    self.assertTrue(b_1 in a.list_b)
AssertionError: False is not true

----------------------------------------------------------------------
Ran 2 tests in 0.017s

FAILED (failures=1)

@JohnAD
Copy link
Contributor

JohnAD commented Dec 23, 2016

I strongly suspect that this issue was related to #1318; which is now resolved. But, unfortunately, the example code above contains a likely-unintentional bug. In testing the iteration, the following line is used:

    for b in a.list_b: pass

This overwrites the original meaning of b. In fact, the for loop would leave the value of b to be b_1. So when:

    a.list_b.remove(b)

is then executed, it is b_1 that is actually removed not the original b. In fact, the original reference to b is lost, so there is no way to remove it with the remove function. By changing the iteration to not overwrite the meaning of b :

    for x in a.list_b: pass

Now the code functions as expected against the latest commit of master. I recommend closing this issue.

@wojcikstefan
Copy link
Member

Thanks for a thorough explanation @JohnAD!

@RyanKung
Copy link
Author

RyanKung commented Jan 6, 2017

Wow, That's my mistake, thanks for pointing it out, @JohnAD

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants