What's the standard way of saving something only if its foreign key exists?

Question

I'm using Python 3.7 and Django . I have the following model, with a foreign key to another model ...

class ArticleStat(models.Model):
    objects = ArticleStatManager()
    article = models.ForeignKey(Article, on_delete=models.CASCADE, related_name='articlestats')
    ...

    def save(self, *args, **kwargs):
        if self.article.exists():
            try:
                article_stat = ArticleStat.objects.get(article=self.article, elapsed_time_in_seconds=self.elapsed_time_in_seconds)
                self.id = article_stat.id
                super().save(*args, **kwargs, update_fields=["hits"])
            except ObjectDoesNotExist:
                super().save(*args, **kwargs)

I only want to save this if the related foreign key exists, otherwise, I've noticed errors result. What's the standard Django/Python way of doing something like this? I thought I read I could use ".exists()" (Check if an object exists), but instead I get an error

AttributeError: 'Article' object has no attribute 'exists'

Edit: This is the unit test I have to check this ...

    id = 1
    article = Article.objects.get(pk=id)
    self.assertTrue(article, "A pre-condition of this test is that an article exist with id=" + str(id))
    articlestat = ArticleStat(article=article, elapsed_time_in_seconds=250, hits=25)
    # Delete the article
    article.delete()
    # Attempt to save ArticleStat
    articlestat.save()

The problem here is that you are calling the `exists()` on a instance and not on a queryset. Please check docs and the examples https://docs.djangoproject.com/en/2.1/ref/models/querysets/#django.db.models.query.QuerySet.exists — Искрен Станиславов, Feb 24 '19 at 17:03

Paolo Melchiorre · Answer 1 · 2019-02-20T08:08:47.453

8

If you want to be sure Article exists in ArticleStat's save method you can try to get it from your database and not just test self.article.

Quoting Alex Martelli:

" ... Grace Murray Hopper's famous motto, "It's easier to ask forgiveness than permission", has many useful applications -- in Python, ... "

I think using try .. except .. else is more pythonic and I will do something like that:

from django.db import models

class ArticleStat(models.Model):
    ...
    article = models.ForeignKey(
        Article, on_delete=models.CASCADE, related_name='articlestats'
    )

    def save(self, *args, **kwargs):
        try:
            article = Article.objects.get(pk=self.article_id)
        except Article.DoesNotExist:
            pass
        else:
            try:
                article_stat = ArticleStat.objects.get(
                    article=article,
                    elapsed_time_in_seconds=self.elapsed_time_in_seconds
                )
                self.id = article_stat.id
                super().save(*args, **kwargs, update_fields=["hits"])
            except ArticleStat.DoesNotExist:
                super().save(*args, **kwargs)

edited Feb 20 '19 at 08:08

answered Feb 18 '19 at 09:31

Paolo Melchiorre

5,716
1
33
52

Thanks for this. Is this the typical way something like this is handled? I ask because in the rare event the article object is deleted after your "article = Article.objects.get(pk=self.ar" and before the "save" line, this method would fail, wouldn't it? – Dave Feb 19 '19 at 15:58
I tried to write an answer based on your question but I have no enough data to say if your solution is the best one to handle this behavior in your particular project. For the "rare events" you can use transactions. – Paolo Melchiorre Feb 19 '19 at 17:28
For the transactional approach, do you have an example of how that would look? – Dave Feb 19 '19 at 17:29
The official documentation is the best starting point https://docs.djangoproject.com/en/stable/topics/db/transactions/#controlling-transactions-explicitly – Paolo Melchiorre Feb 19 '19 at 17:31
1

I have seen others point out on SO that links tend to break over time and so embedded answers are preferable in questions. You don't have to add anything to your answer but some kind of efficient, foolproof code woudl definitely be what I'm looking for. – Dave Feb 19 '19 at 17:52
1

I read again your comment and I confirm you the code a proposed in my answer. "In the rare event the article object is deleted the get method of ArticleStat will fail but the except will handle the failure. So you don't need any transaction as I wrote in the comments. – Paolo Melchiorre Feb 20 '19 at 08:11

score 5 · Answer 2 · answered Feb 18 '19 at 22:32

5

If you are using a relational database, foreign key constraints will be added automatically post-migration. save method may not need any customization.

class ArticleStat(models.Model):
    objects = ArticleStatManager()
    article = models.ForeignKey(
        Article, on_delete=models.CASCADE, related_name='articlestats'
    )

Use the following code to create ArticleStats

from django.db import IntegrityError
try:
  ArticleStats.objects.create(article=article, ...)
except IntegrityError:
  pass

If article_id is valid, ArticleStats objects get created else IntegrityError is raised.

article = Article.objects.get(id=1)
article.delete()
try:
  ArticleStats.objects.create(article=article, ...)
  print("article stats is created")
except IntegrityError:
  print("article stats is not created")


# Output
article stats is not created

Note: Tested on MySQL v5.7, Django 1.11

answered Feb 18 '19 at 22:32

sun_jara

1,736
12
20

2

I wrote a unit test to check this (included now in the question), but using your code still results in an error, "ValueError: save() prohibited to prevent data loss due to unsaved related object 'article'." Maybe taht's because the unit tests uses SqlLite? – Dave Feb 19 '19 at 15:56
1

Foreign key constraints are disabled in sqlite by default, but that's not the reason for this error. Irrespective of the database backend you will get this error. Check [this](https://stackoverflow.com/questions/33838433/save-prohibited-to-prevent-data-loss-due-to-unsaved-related-object) link. – sun_jara Feb 19 '19 at 17:24
1

Instead of using .save() if you use .create(), then it will never raise this error. suggestion: articlestat = ArticleStat.objects.create(article=article, elapsed_time_in_seconds=250, hits=25) (.create() internally calls .save()) – sun_jara Feb 19 '19 at 17:27
1

What about updates to an object? I tried replacing "save" with "Update" but got an error, "AttributeError: 'super' object has no attribute 'update'" – Dave Feb 19 '19 at 17:33
1

.update() is defined in QuerySet and will work only with queryset. For example, .update() works with ArticleStat.objects.al().update(sometime) or ArticleStat.objects.filter().update() But will not work with ArticleStat.objects.get(id=id).update() <- this will throw error. – sun_jara Feb 19 '19 at 17:41
The foreign key constrains are at database level, so either you use .update() or .create() or .save(), you will get IntegrityError if the article.id is invalid. – sun_jara Feb 19 '19 at 17:44
The "update" i'm referring to is changing my line to look like this "super().update(*args, **kwargs, update_fields=["hits"])" – Dave Feb 19 '19 at 17:55
Got it. .update() method is not part of Model. Hence you got that error when you replaced it .save() with .update() in your code. I recommend you not override the save method. If you are using any RDBMS as a database backend, you will never have any issue with data integrity, i.e. if an article is not in Article table, it can never be referred in ArticleStats. In fact, if you try to do it in application code, Django will throw integrity error. 1/2 – sun_jara Feb 20 '19 at 10:00
Not only this, if you go directly into the database and try to delete the article table, you will not be able to do it. The database software will throw an error saying there is another table called ArticleStats that's using Article. That's the beauty of RDBMS databases. 2/2 – sun_jara Feb 20 '19 at 10:00
Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/188741/discussion-between-sun-jara-and-dave). – sun_jara Feb 20 '19 at 10:00

score 5 · Answer 3 · answered Feb 19 '19 at 02:05

article field on your ArticleStat model is not optional. You can't save your ArticleStat object without the ForeignKey to Article

Here is a similar code, item is a ForeignKey to the Item model, and it is required.

class Interaction(TimeStampedModel, models.Model):
    ...
    item = models.ForeignKey(Item, on_delete=models.CASCADE, related_name='interactions')
    type = models.IntegerField('Type', choices=TYPE_CHOICES)
    ...

If I try to save an object of Interaction from the shell without selecting a ForeignKey to the item, I receive an IntegrityError.

~ interaction = Interaction()
~ interaction.save()
~ IntegrityError: null value in column "item_id" violates not-null constraint

You don't need a check self.article.exists(). Django and Database will require that field and will not let you save the object without it.

You should read about ForeignKey field in Django Docs

Hi, I realize Django won't let me save the field if hte article doesn't exist -- an error gets thrown, which halts my program. I don't want my program to halt. What's the typical Django way for dealing with this kind of thing? It seems like this must be a pretty common thing for programmers to experience and Django has some slick way of dealing with it. — Dave, Feb 23 '19 at 19:41

Jonah Bishop · Answer 4 · 2019-02-14T02:23:53.087

4

You can just test the value of the article field. If it's not set, I believe it defaults to None.

if self.article:  # Value is set

If you want this ForeignKey field to be optional (which it sounds like you do), you need to set blank=True and null=True on that field. This will allow the field to be blank (in validation) and will set null on the field when it's not there.

As mentioned in the comments below, your database is likely enforcing the fact that the field is required, and refuses to remove the article instance.

edited Feb 14 '19 at 02:23

answered Feb 14 '19 at 02:03

Jonah Bishop

12,279
6
49
74

If you don't need the article instance and just want to check for existence, I'd refine this to `if self.article_id:`. This then avoids a database query being executed if `article` wasn't previously fetched. – lukewarm Feb 14 '19 at 02:10
@JonahBishop, "if self.article:" doesn't work. Even if the article has already been deleted from the database, "if self.article" still evalutes to a non-null entity. – Dave Feb 14 '19 at 02:15
@lukewarm, "if self.article_id:" also seems to evalute to true even fi the article has been deleted from the database. – Dave Feb 14 '19 at 02:17
If you change the model to include the `null=True` option, does that change the effects of deleting the model instance? – Jonah Bishop Feb 14 '19 at 02:19
1

@dave are you certain the Article instance has been deleted? Most databases will enforce referential integrity to some extent, making that scenario unlikely if you've just used regular `ForeignKey` fields. Your response to @JonahBishop also seems to suggest that the Article row has, in fact, not been deleted .. – lukewarm Feb 14 '19 at 02:21
@lukewarm, I know the Article has been deleted because when the "super().save" line is executed, it results in an "ValueError: save() prohibited to prevent data loss due to unsaved related object 'article'." error in my unit test. – Dave Feb 14 '19 at 02:35
@JonahBishop, where am I putting "null=True"? Can you list it out in your answer? – Dave Feb 14 '19 at 02:35
Also "on_delete" needs to be set to "models.SET_NULL". – birophilo Feb 24 '19 at 13:57

birophilo · Answer 5 · 2019-02-24T13:50:05.610

3

As other answers have pointed out, the Article ForeignKey is required on your ArticleStat model and saving will automatically fail without a valid Article instance. The best way to fail gracefully with non-valid input is using Form validation with Django's Forms API. Or if handling serialized data with Django Rest Framework, using a Serializer, which is the Form-like equivalent for JSON data. That way you don't need to overwrite the save method unless you have a specific requirement.

So far nobody has mentioned the correct usage of .exists(). It is a method of a queryset not a model instance, which is why you get the error you mentioned above when trying to apply it to an individual model instance with self.article.exists(). To check the existence of an object, simply use .filter instead of .get. If your Article (pk=1) exists then:

Article.objects.filter(pk=1)

Will return a queryset with one Article in it:

<Queryset: [Article: 1]>

and

Article.objects.filter(pk=1).exists()

Will return True. Whereas if the item does not exist the query will return an empty queryset and .exists() will return False, rather than raising an exception (as attempting to .get() a non-existent object does). This still applies if the pk previously existed and has been deleted.

EDIT: Just noticed that your ArticleStat's on_delete behaviour is currently set to CASCADE. This means that when an Article is deleted, the related ArticleStat is also deleted. So I think you must have been misinterpreting the errors/difficulties you mentioned in reply to @jonah-bishop's answer when trying if self.article:. For a minimal fix, if you still want to keep the ArticleStat after an Article is deleted, change the on_delete keyword to models.SET_NULL, and as per Jonah's answer, add the extra keywords null=True, blank=True:

article = models.ForeignKey(Article, on_delete=models.SET_NULL, related_name='articlestats', null=True, blank=True)

Then there should be no problem simply doing if self.article: to check for a valid ForeignKey object relation. But using Forms/Serializers is still better practice though.

edited Feb 24 '19 at 13:50

answered Feb 22 '19 at 18:41

birophilo

912
10
20

So with regards to writing my "save" method, where am I putting the line "Article.objects.filter(pk=1).exists()"? – Dave Feb 23 '19 at 19:14
You can use that for your unit test check, but it's better not to write a custom save method on your model just to check for a valid Article. In Django, it's highly recommended practice to process all user input through a Form (or Serializer) before saving, as it not only validates the data, but provides built-in protection against malicious intent. The word "Form" doesn't necessarily entail manual user input by the way - a Django Form is just an object that validates content. A `ModelForm` can validate all model fields with just a few words of code. – birophilo Feb 23 '19 at 19:58
You pass the input data into the form then call `form.is_valid()`, which will catch exceptions and return a `False` value if there are, so you only save it if `True`. And if you use Django's class-based views, you don't have to write anything except the form and assign it to your view. Check out the docs; also this is a [good Form walkthrough](https://tutorial.djangogirls.org/en/django_forms/). – birophilo Feb 23 '19 at 20:00
Thanks but I'm not using forms. Objects are getting created through a service run as a Python command (sometimes multiople commands run simultaneously, leading to these weird situations where objects may not exist). – Dave Feb 23 '19 at 21:50
They'll work for that use case too. A Django form/serializer is just an object that does the hard work of validating data and handling exceptions before a save. Otherwise you will have to catch the exception yourself because you are either selecting something that isn't there or leaving a required database field blank. (Unless you have access to the expected pk of the Article from elsewhere in your process, in which case you could run a single item filter as above.) – birophilo Feb 24 '19 at 00:42
(in case of confusion: you don't need any front end code or HTTP submission for a "Form", you can just populate it with your own data on the back end) – birophilo Feb 24 '19 at 00:53
Thanks. So if I'm reading your edited answer correctly, the thing I'm missing is adding "if self.article" before I save? Unfortunately, this doesn't work. I'm noticing "if self.article" is returning true even when its run directly after an "article.delete()" statement. If I've just deletd the article, it shoudl return false, right? At least, I would like some kind of test to tell me the article no longer exists. – Dave Feb 24 '19 at 18:18

score 0 · Answer 6 · answered Feb 21 '19 at 08:18

The code of last line articlestat.save() will fail if the article instance has been deleted. Django and database will check the article automatically for you if you are using relation database like mysql or sqlite3.

During the migrations, a constraint will be created. For example:

shell>>> python manage.py sqlmigrate <appname> 0001
CREATE TABLE impress_impress ...
...

ALTER TABLE `impress_impress` ADD CONSTRAINT 
    `impress_impress_target_id_73acd523_fk_account_myuser_id` FOREIGN KEY (`target_id`) 
    REFERENCES `account_myuser` (`id`);

...

So if you want to save the articlestat without article, an error will be raised.

Hi, Yes, I know an error will be raised if the article doesn't exist -- that's why I'm asking my question. How do I prevent a call to the save if teh article doesn't exist or at least make everythign fail gracefully? I just don't want any exception being thrown from that method. — Dave, Feb 21 '19 at 15:24

score 0 · Answer 7 · answered Feb 23 '19 at 15:29

0

You can call .full_clean() before .save()

from django.core.exceptions import ValidationError

class ArticleStat(models.Model):
    #...
    def save(self, *args, **kwargs):
        try:
            self.full_clean()
        except ValidationError as e:
            # dont save
            # Do something based on the errors contained in e.message_dict.
            # Display them to a user, or handle them programmatically.
            pass
        else:
            super().save(*args, **kwargs)

answered Feb 23 '19 at 15:29

Todor

15,307
5
55
62

Hi, THis doesn't work. When I call the "save" method for the purposes of updating an existing object, the ValidationError gets called with the error, '['Article stat with this Article and Elapsed time in seconds already exists.']}' – Dave Feb 23 '19 at 19:13
Search for errors in your code, `full_clean` knows how to handle update operations. – Todor Feb 23 '19 at 21:30
My update an existing object code was working fine, but when I added the "self.full_clean()" stuff, I got the error in my previous comment. I have a unique constraint on the two fields mentioned in the error. Maybe full_clean doesn't work with unique constraints for updates? – Dave Feb 23 '19 at 21:54
Read the [docs](https://docs.djangoproject.com/en/2.1/ref/models/instances/#django.db.models.Model.full_clean), check the [source](https://docs.djangoproject.com/en/2.1/_modules/django/db/models/base/#Model.full_clean) code and maybe you will see that it support [update](https://github.com/django/django/blob/2.1.7/django/db/models/base.py#L1032) operations and will find the error in your code. – Todor Feb 24 '19 at 00:19

What's the standard way of saving something only if its foreign key exists?

7 Answers7