GoogleCloudPlatform / endpoints-proto-datastore

Apache License 2.0
154 stars 52 forks source link

NotFoundException raised when query filter value for a KeyProperty matches no entities #152

Closed tmst closed 7 years ago

tmst commented 7 years ago

If MyModel has AliasPropertyattr_id, a query filter which specifies a value for attr1_id will fail if no corresponding Attr entity exists in the datastore (attr is a KeyProperty):

query.filter(MyModel.attr_id = 'attr1') =>
        NotFoundException("No attr found with key ID 'attr1'")

The query should correctly return an empty list as it does with any other property filter having no matches.

BTW, I observe this behavior in API Explorer. I'm not certain that it's something that can't be handled on the application side in the method AttrIdSet with the following signature:

def AttrIdSet(self, value):
    if not isinstance(value, basestring):
        raise TypeError('ID must be a string')
    # Verify the Attr exists
    attr = Attr.get_by_id(value)
    if not attr:
        raise endpoints.NotFoundException(
                "No attr found with key '%s'" % value)
    self.attr = attr.key

The problem, of course, is that this method gets invoked for all HTTP methods, not just queries. When inserting or deleting an entity it must be verified that the entity exists, whereas with queries, we're only interested in whether a MyModel instance exists having an attr_id == 'attr1'`.

dhermes commented 7 years ago

@tmst Do you have a bare-bones sample app I could run while digging into this?

tmst commented 7 years ago

I'm not sure how to run the test suite for the EPD examples. But the "keys_with_ancestors" example seems to display this problem:

  1. Create a MyParent myparent1
  2. Create a MyModel mymodel1
  3. List MyModel, passing in 'myparent2' as the parent => "404 - Parent myparent2 does not exist."
dhermes commented 7 years ago

:+1: Thanks i'll look into it

dhermes commented 7 years ago

@tmst That example app does as you mentioned:

$ curl http://localhost:8080/_ah/api/myapi/v1/mymodels/myparent2
{
 "error": {
  "code": 404, 
  "errors": [
   {
    "domain": "global", 
    "message": "Parent myparent2 does not exist.", 
    "reason": "notFound"
   }
  ], 
  "message": "Parent myparent2 does not exist."
 }
}

However, the 404 is not from the library, but rather from the "user code" of the app.

tmst commented 7 years ago

Right. I touched upon this fact in my original question. The trouble is that, due to the design of EPD AliasProperty, it seems unavoidable that the application code be written in this manner. Specifically, that the existence of a parent entity be verified in every HTTP method operation on a child Kind, regardless of whether a new child entity is being added to the datastore, or the child entities are simply being listed.

I suppose it's sort of nit-picky issue. In most use cases the parent will be populated in the request from a list of parents on the client, and if a nonexistent parent is sent in the query then another problem also exists.

dhermes commented 7 years ago

Yeah the parent/child thing is not a builtin part of the library, was just my imagination for something you might build on top of it.

tmst commented 7 years ago

Thanks. I'm fine with this issue being closed. StackOverflow is probably a better place to find out how a query might be written along the lines of the "keys_with_ancestors" example but in which the existence of parents is not verified.

dhermes commented 7 years ago

Sorry I couldn't do more. Cheers. (Again, thanks for taking the time to engage.)