GoogleCloudPlatform / appengine-python-standard

Google App Engine services SDK for Python 3
Apache License 2.0
70 stars 47 forks source link

`StringProperty` value of `StructuredProperty` is coerced to `bytes` during `choices` validation β†’ BadValueError #104

Open blech75 opened 1 year ago

blech75 commented 1 year ago

Expected Behavior

Given the following example ndb models and code:

from google.appengine.ext import ndb

RED = "red"
GREEN = "green"
BLUE = "blue"

COLORS = [RED, GREEN, BLUE]

class Car(ndb.Model):
    name = ndb.StringProperty(required=True)
    color = ndb.StringProperty(choices=COLORS, required=True)

class Dealership(ndb.Model):
    cars = ndb.StructuredProperty(Car, repeated=True)

d1 = Dealership(cars=[Car(name="Ferrari", color=RED)]).put().get()
print(d1)

d2 = Dealership.query(Dealership.cars == Car(color=RED)).get()
print(d2)

print(str(d1 == d2))
assert d1 == d2

...we should see:

Dealership(key=Key('Dealership', 1), cars=[Car(color='red', name='Ferrari')])
Dealership(key=Key('Dealership', 1), cars=[Car(color='red', name='Ferrari')])
True

...and there are no errors. This works fine in Python 2 with the Google Cloud SDK. πŸ‘

Actual Behavior

Using Python 3.9.16 and appengine-python-standard, however, we see:

BadValueError: Value b'red' for property b'cars.color' is not an allowed choice`

...which is confusing because 'red' was provided as the value, not b'red'. πŸ€”πŸ€”πŸ€”

Digging around a bit, it appears to fail in StructuredProperty._comparison() (triggered by the Dealership.cars == Car(color=RED) expression), specifically right here:

https://github.com/GoogleCloudPlatform/appengine-python-standard/blob/882b8fa098220636a1c259a1dfaa1e307bcfcf72/src/google/appengine/ext/ndb/model.py#L2381-L2382

...because the provided color value is converted to a _BaseValue() of bytes via Property._get_base_value_unwrapped_as_list() β†’ Property._get_base_value() β†’ Property._opt_call_to_base_type() β†’ Property._apply_to_values() β†’ TextProperty._to_base_type():

https://github.com/GoogleCloudPlatform/appengine-python-standard/blob/882b8fa098220636a1c259a1dfaa1e307bcfcf72/src/google/appengine/ext/ndb/model.py#L1831-L1833

...before being compared to the allowed choices (which are str). 😞

A workaround is to adjust the values in choices to be of type bytes:

RED = b"red"
GREEN = b"green"
BLUE = b"blue"

...and we then see the same result:

Dealership(key=Key('Dealership', 1), cars=[Car(color='red', name='Ferrari')])
Dealership(key=Key('Dealership', 1), cars=[Car(color='red', name='Ferrari')])
True

(This also works fine in Python 2 with the Google Cloud SDK. πŸ˜…)

Steps to Reproduce the Problem

(see code above)

Specifications


Additional Info

This does not appear to have anything to do with persisting data -- the persisted value of Dealership.cars[].color remains a str: type(d1.cars[0].color) == str.

It only happens during "comparison" (a key part of querying) when the model with a StringProperty(choices=...) is a StructuredProperty of another model. . We can see this by skipping entity creation and just calling Dealership.query(Dealership.cars == Car(color=BLUE)), or even Dealership.cars == Car(color=BLUE) as the most minimal case.

When using a standalone model, all works fine:

ferrari = Car(name="Ferrari", color=RED).put().get()
print(ferrari)

red_car = Car.query(Car.color == RED).get()
print(red_car)

print(str(ferrari == red_car))
assert ferrari == red_car

...yields:

Car(key=Key('Car', 1), color='red', name='Ferrari')
Car(key=Key('Car', 1), color='red', name='Ferrari')
True