Fix re-use of indexes and refresh method on null attributes

[jhh3000]

I'd like to contribute a fix for what I think is a bug (but may be intended design) for re-use of indexes. If you define an index class and use it in two different models, the object returned will always be the object type of the last-bound model. This is due to the fact that .model is being bound to the Meta class of the index, and not an instance of the class. To solve this, I bind the model to the instance of the index and change the methods to be bound to the index instance rather than class.

Also, there's a fix in here for calling Object.refresh(), where if the object has a null value for a non-nullable field, it won't ever refresh, even though the DB may have fresh data.

I wasn't able to figure out how to run integration tests (seems to be hitting a DynamoDB AWS table) or signal tests, but the model tests pass.

Comments welcome! Thanks.

[ikonst]

Makes total sense to me.

@conjmurph did you run into this same refresh issue?

[conjmurph]

Makes total sense to me.

@conjmurph did you run into this same refresh issue?

I ran into an issue where the response from an UpdateItem request was not properly used to update the in memory model. This issue is kind of similar in that the logic for a Model.get and Model.refresh are not in sync, but the error seems to occur before we execute the request to dynamodb. I haven't taken too close a look at this PR, but the refresh bit seems good. Maybe we take it a step further and ensure Model.get and Model.refresh share more logic e.g.

class Model:

    @classmethod
    def _get_item_or_raise(
        cls,
        hash_key: _KeyType,
        range_key: _KeyType,
        consistent_read: bool = False,
        attributes_to_get: Optional[Sequence[Text]] = None,
    ) -> Dict:
        data = cls._get_connection().get_item(
            hash_key=hash_key,
            range_key=range_key,
            consistent_read=consistent_read,
            attributes_to_get=attributes_to_get
        )
        if not data or ITEM not in data:
            raise cls.DoesNotExist()
        return data

    @classmethod
    def get(
        cls: Type[_T],
        hash_key: _KeyType,
        range_key: Optional[_KeyType] = None,
        consistent_read: bool = False,
        attributes_to_get: Optional[Sequence[Text]] = None,
    ) -> _T:
        """
        Returns a single object using the provided keys

        :param hash_key: The hash key of the desired item
        :param range_key: The range key of the desired item, only used when appropriate.
        :param consistent_read:
        :param attributes_to_get:
        :raises ModelInstance.DoesNotExist: if the object to be updated does not exist
        """
        hash_key, range_key = cls._serialize_keys(hash_key, range_key)
        data = cls._get_item_or_raise(
            hash_key=hash_key,
            range_key=range_key,
            consistent_read=consistent_read,
            attributes_to_get=attributes_to_get
        )
        return cls.from_raw_data(data[ITEM])

    def refresh(self, consistent_read: bool = False) -> None:
        """
        Retrieves this object's data from dynamodb and syncs this local object

        :param consistent_read: If True, then a consistent read is performed.
        :raises ModelInstance.DoesNotExist: if the object to be updated does not exist
        """
        hash_key = self._hash_key_attribute()
        range_key = self._range_key_attribute()
        data = self._get_item_or_raise(
            hash_key=hash_key,
            range_key=range_key,
            consistent_read=consistent_read,
        ) 
        self._deserialize(data[ITEM])

[jhh3000]

Just to be clear, the .refresh() issue is caused by how we're using the ORM. We're querying against a GSI with a different SK that stores object-relation information. Then, we iterate through the result set and correct the SK and call .refresh().

But in general, this pattern fails:

obj = Object.get(id)
obj.some_attr = None
obj.refresh()

We're using a single, de-normalized table, which is how we're discovering these issues.