Skip to content
This repository was archived by the owner on May 6, 2026. It is now read-only.

fix: prevent mismatch error with default namespace on ancestor queries #604

Closed
cguardia wants to merge 1 commit into from
Closed

fix: prevent mismatch error with default namespace on ancestor queries #604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion google/cloud/ndb/query.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1392,7 +1392,13 @@ def __init__(
else:
project = ancestor.app()
if namespace is not None:
if namespace != ancestor.namespace():
# if namespace is the empty string, that means default
# namespace, but after a put, if the ancestor is using
# the default namespace, its namespace will be None,
# so skip the test to avoid a false mismatch error.
if namespace == "" and ancestor.namespace() is None:
pass
elif namespace != ancestor.namespace():
raise TypeError("ancestor/namespace mismatch")
else:
namespace = ancestor.namespace()
Expand Down
50 changes: 50 additions & 0 deletions tests/system/test_query.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,56 @@ class SomeKind(ndb.Model):
assert [entity.foo for entity in results] == [-1, 0, 1, 2, 3, 4]


def test_ancestor_query_with_namespace(client_context, dispose_of, other_namespace):
class Dummy(ndb.Model):
foo = ndb.StringProperty(default="")

entity1 = Dummy(foo="bar", namespace="xyz")
parent_key = entity1.put()
dispose_of(entity1.key._key)

entity2 = Dummy(foo="child", parent=parent_key, namespace=None)
entity2.put()
dispose_of(entity2.key._key)

entity3 = Dummy(foo="childless", namespace="xyz")
entity3.put()
dispose_of(entity3.key._key)

with client_context.new(namespace=other_namespace).use():
query = Dummy.query(ancestor=parent_key, namespace="xyz")
results = eventually(query.fetch, length_equals(2))

assert results[0].foo == "bar"
assert results[1].foo == "child"


def test_ancestor_query_with_default_namespace(
client_context, dispose_of, other_namespace
):
class Dummy(ndb.Model):
foo = ndb.StringProperty(default="")

entity1 = Dummy(foo="bar", namespace="")
parent_key = entity1.put()
dispose_of(entity1.key._key)

entity2 = Dummy(foo="child", parent=parent_key, namespace=None)
Copy link
Copy Markdown
Contributor

@chrisrossi chrisrossi Feb 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed in the customer's example that they don't explicitly pass namespace. I would think that the namespace would be implied by passing in parent. For grins, I tried removing the namespace arg from this test and I get a namespace mismatch exception in the model constructor. Is there maybe a little more work to do here?

Copy link
Copy Markdown
Contributor Author

@cguardia cguardia Feb 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I confess I added this to make the test pass, but the customer's code does run unchanged if using a python script outside of the test machinery. Maybe I'm missing something on the test setup?

Copy link
Copy Markdown
Contributor

@chrisrossi chrisrossi Feb 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like it's just because in the customer's example the context's namespace is still the default namespace when that entity is created, so there won't be a mismatch between entity2 and its parent. Does still seem like specifying parent should imply the namespace, though.

I know you're starting your new job. Would you like me to take this one?

Copy link
Copy Markdown
Contributor Author

@cguardia cguardia Feb 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please take this. I appreciate the offer.

FWIW, the following code caused the mismatch error before the fix was applied, and it works fine with the fix in:

import os
from google.cloud import ndb

class Dummy(ndb.Model):
    foo = ndb.StringProperty(default='')

with ndb.Client().context():
    entity1 = Dummy(foo='bar', namespace='')
    entity1.put()
    key_entity1 = entity1.key

    entity2 = Dummy(foo='child', parent=key_entity1)
    entity2.put()

    entity3 = Dummy(foo='childless')
    entity3.put()

with ndb.Client().context(namespace='other_context'):
    l = Dummy.query(ancestor=key_entity1, namespace='').fetch()
    for e in l:
        print(e)

I couldn't quite duplicate that in the text.

entity2.put()
dispose_of(entity2.key._key)

entity3 = Dummy(foo="childless", namespace="")
entity3.put()
dispose_of(entity3.key._key)

with client_context.new(namespace=other_namespace).use():
Copy link
Copy Markdown
Contributor

@chrisrossi chrisrossi Feb 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, tests are not, by default, run in the default namespace, so you don't really have to switch namespace here. It's already not the default. Although, I guess it might make it more readable to show that the namespace is different explicitly.

query = Dummy.query(ancestor=parent_key, namespace="")
results = eventually(query.fetch, length_equals(2))

assert results[0].foo == "bar"
assert results[1].foo == "child"


@pytest.mark.usefixtures("client_context")
def test_projection(ds_entity):
entity_id = test_utils.system.unique_resource_id()
Expand Down
7 changes: 7 additions & 0 deletions tests/unit/test_query.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,13 @@ def test_constructor_with_ancestor_and_namespace():
query = query_module.Query(ancestor=key, namespace="space")
assert query.namespace == "space"

@staticmethod
@pytest.mark.usefixtures("in_context")
def test_constructor_with_ancestor_and_default_namespace():
key = key_module.Key("a", "b", namespace=None)
query = query_module.Query(ancestor=key, namespace="")
assert query.namespace == ""

@staticmethod
@pytest.mark.usefixtures("in_context")
def test_constructor_with_ancestor_parameterized_thing():
Expand Down