Model.refresh_from_db raises DoesNotExist

[AndreasBackx]

Calling Model.refresh_from_db on a soft deleted model will raise a Model.DoesNotExist exception. Should this be the expected behaviour? I could look into this and make a PR. But from guessing I would say it might require changing the behaviour of the QuerySet set as the default by django-safedelete. I really like how django-permanent distinguishes between showing all of the objects, the non-deleted objects, and even only the deleted objects.

>>> MyModel.objects.count()
2
>>> a.delete()
>>> MyModel.objects.count()
1
>>> MyModel.deleted_objects.count()
1
>>> MyModel.all_objects.count()
2
>>> MyModel._base_manager.count()
2

Maybe this behaviour is something more desirable that we should look into? Perhaps even implement both (all_with_deleted and the custom managers like in django-permanent).

[Gagaro]

It shouldn't be too hard to add those managers (with 0.4.x anyway), if you make a PR I'll gladly merge it. Having several managers seems closer to the Django philosophy in this case indeed. I'm not sure if with should keep the all_with_deleted if we do that though.

[ddahan]

If this can help, as a workaround, I replaced refresh_from_db with a manual reload that works for any object that inherits from SafeDeleteModel. Here is how I did:

from django.apps import apps
model = apps.get_model(obj._meta.app_label, obj._meta.model_name)
obj = model.all_objects.get(pk=obj.pk)
# Now, obj has ben refreshed from the db

[aljp]

My solution to this has been a abstract model class (should perhaps be a mixin) that adds a refresh_from_manager method to my models, almost identical to refresh_from_db except it accepts a manager_name: Optional[str]=None kwarg to specify the manager to use for the refresh. A PR could override SafeDeleteModel.refresh_from_db and add the manager_name kwarg like in the snippet below.

class RefreshFromManagerModelMixin(models.Model):
    """This class adds a method to models which fix an issue with the django-safedelete library and
    the `refresh_from_db` method."""

    def refresh_from_manager(self, using=None, fields=None, manager_name=None):
        """
        Reloads field values from the database using a specific manager on the model class.

        By default, the reloading happens from the database this instance was
        loaded from, or by the read router if this instance wasn't loaded from
        any database. The using parameter will override the default.

        Fields can be used to specify which fields to reload. The fields
        should be an iterable of field attnames. If fields is None, then
        all non-deferred fields are reloaded.

        When accessing deferred fields of an instance, the deferred loading
        of the field will call this method.
        """
        if fields is not None:
            if len(fields) == 0:
                return
            if any(LOOKUP_SEP in f for f in fields):
                raise ValueError(
                    'Found "%s" in fields argument. Relations and transforms '
                    "are not allowed in fields." % LOOKUP_SEP
                )

        db = using if using is not None else self._state.db

        if manager_name:
            if hasattr(self.__class__, manager_name) and isinstance(
                getattr(self.__class__, manager_name), Manager
            ):
                model_manager = getattr(self.__class__, manager_name)
            else:
                logger.warning(
                    f"The {manager_name} property is either not an attribute or not an instance of Manager"
                )
        else:
            model_manager = self.__class__._default_manager

        db_instance_qs = model_manager.using(db).filter(pk=self.pk)

        # Use provided fields, if not set then reload all non-deferred fields.
        deferred_fields = self.get_deferred_fields()
        if fields is not None:
            fields = list(fields)
            db_instance_qs = db_instance_qs.only(*fields)
        elif deferred_fields:
            fields = [
                f.attname
                for f in self._meta.concrete_fields
                if f.attname not in deferred_fields
            ]
            db_instance_qs = db_instance_qs.only(*fields)

        db_instance = db_instance_qs.get()
        non_loaded_fields = db_instance.get_deferred_fields()
        for field in self._meta.concrete_fields:
            if field.attname in non_loaded_fields:
                # This field wasn't refreshed - skip ahead.
                continue
            setattr(self, field.attname, getattr(db_instance, field.attname))
            # Throw away stale foreign key references.
            if field.is_relation and field.get_cache_name() in self.__dict__:
                rel_instance = getattr(self, field.get_cache_name())
                local_val = getattr(db_instance, field.attname)
                related_val = (
                    None
                    if rel_instance is None
                    else getattr(rel_instance, field.target_field.attname)
                )
                if local_val != related_val or (
                    local_val is None and related_val is None
                ):
                    del self.__dict__[field.get_cache_name()]
        self._state.db = db_instance._state.db

    class Meta:
        abstract = True