3
\$\begingroup\$

This is an extension of this question due to it not covering an important role of my setup that I think needs more attention.

The files are stored on a different server for separation and convenience. I'm trying to store references to files in my REST API.

I'm using Django for my REST API and I need to find a convenient, practical and quick way to store database references such as images and audio. The features that my implementation supports are as follows:

  • File sets: These make it possible to store several files that are of different size, but represent the same content. (Thumbnails for example)
  • Unique UUID for the filesets (also used to check if a user can access the file)
  • The models are generic (Meaning they can be applied to any model)
  • Some media can be optional

I would like to keep these features but at the same time keep my code readable and have it make sense.

The Media models look like this:

class Media(models.Model):
    # Identifiers
    user = models.ForeignKey(User, unique=False, related_name = 'media')
    folder = models.CharField(max_length=100, blank=True)
    uid = models.CharField(max_length=255, unique=True)

    # Resource
    content_type = models.ForeignKey(ContentType, blank=True, null=True)
    object_id = models.PositiveIntegerField(blank=True, null=True)
    content = GenericForeignKey('content_type', 'object_id')

    # Other
    date_created = models.DateTimeField(auto_now=False, auto_now_add=True, blank=True)

    class Meta:
         unique_together = ('content_type', 'object_id',)

    def __str__(self):
        return self.uid

class File(models.Model):
    name = models.CharField(max_length=100, blank=True)
    extension = models.CharField(max_length=100, blank=True)

    class Meta:
        abstract = True

# Image File
class Image(File):
    parent = models.ForeignKey(Media, related_name='images', null=True)

    # Dimensions
    width = models.CharField(max_length=100, blank=True)
    height = models.CharField(max_length=100, blank=True)

# Audio file
class Audio(File):
    parent = models.ForeignKey(Media, related_name='audio', null=True)

    # Technical
    duration_seconds = models.FloatField(blank=True,default=0)
    duration = models.CharField(blank=True, max_length=100)

    bitrate = models.CharField(blank=True, max_length=100)
    channels = models.IntegerField(blank=True, null=True)

    def __str__(self):
        return self.name

The file set here is the Media model storing the uuid, the folder the files are located in and which user it belongs to, as well as the generic relation. It can have several files related to it, such as images (again, for thumbnails as an example)

Now, here's where the implementation shows its ugly side. These also have to be assigned to models in order to have any meaning of course. Here's what a model has to do to be able to use the media:

class Post(models.Model):
    ...

    def __str__(self):
        return self.title

class PostImage(models.Model):
    media = GenericRelation('Media.Media')
    resource = models.OneToOneField('Post', null=True, related_name = 'image')

Think that looks fine? Well, the issue really appears when assigning the media to an object and then retrieving it (so the api can give you the url/uris to it)

Inside Media.utilities I have this:

def attach_media(obj, resource, mediaid):
    obj, created = obj.objects.get_or_create(resource=resource)

    media = Media.objects.get(pk=mediaid)
    media.content = obj
    media.save()

def media_exists(id, folder):
    try:
        Media.objects.get(pk=id, folder=folder)
        return True
    except Media.DoesNotExist:
        return False

And then in a view for posting I've got this:

class PostList(APIView):

    def post(self, request):
        data = request.data
        post = Post(...)
        post.save()

        attach_media(PostImage, post, data.get('image'))

        serializer = PostSerializer(post)
        return Response(serializer.data)

As you can see this is a very "ugly" way of doing it, there is no way to re-attach a different image to the post without having to delete the first one somehow due to the unique together constraint.

But it gets worse...

Now that the image has been attached to the post, how do we get it in our client? My serializer looks like this:

in Media.serializers

class FileSerializer(serpy.Serializer):
    url = serpy.MethodField()
    name = serpy.Field()

    def get_url(self, obj):
        return '%s_%s_%s.%s' % (obj.parent.folder, obj.parent.uid, obj.name, obj.extension)

in Post.serializers

class PostSerializer(serpy.Serializer):
    ...

    media = serpy.MethodField()


    def get_media(self, obj):

        if obj.image:
            image = FileSerializer(obj.image.media.all()[0].images.all(), many=True)

        result = {
            'artwork': art.data or None,
        }

        return result

and in a JSON response from the API, it looks something like this:

enter image description here

In conclusion, while this works, it's very ugly and very complicated. I feel like there is a better way to do this.

In addition, this is really really slow. Loading a list of "Posts" take around 700ms to 900ms depending on how many there are. prefetching the media has helped a little, but even then it's still really slow. Removing the media makes the speed go up to 60ms - 100ms

There has to be a better way to do this.

\$\endgroup\$
6
  • 1
    \$\begingroup\$ I'm not entirely sure it can help but: did you consider django's FileField and ImageField? If yes, why couldn't it be an option? \$\endgroup\$ Commented Oct 22, 2016 at 16:33
  • \$\begingroup\$ No. I explicitly explained at the top that I'm not using Django to store files. I'm using a separate server. \$\endgroup\$ Commented Oct 22, 2016 at 17:17
  • \$\begingroup\$ Never tried it, but there might be a way to use a Storage class of your own that tell django to put/retrieve your files to/from the other server. \$\endgroup\$ Commented Oct 22, 2016 at 18:23
  • \$\begingroup\$ I'm not trying to store the files. I'm storing the references. The file server is responsible for storing the files, and then telling django to create a media object that my models then can use. \$\endgroup\$ Commented Oct 22, 2016 at 18:54
  • \$\begingroup\$ OK, my bad, didn't understood the flow was this way. You may want to add this precision into the post, though, in case I wasn't the only one being mislead. \$\endgroup\$ Commented Oct 22, 2016 at 18:58

1 Answer 1

4
+100
\$\begingroup\$

This part here is strange:

obj.image.media.all()[0].images.all()

If by design, Post.image only links to media holding images, why would you not write this as a single query instead?

obj.image.media.images.all()

Now obj might even be an arbitrary QuerySet of the Post model instead of a single post, and you get the run time down to what you originally wanted. Cutting down on the number of individual database queries is the key here.


content = GenericForeignKey('content_type', 'object_id')
class Meta:
     unique_together = ('content_type', 'object_id',)

That one is really weird as well. Why would you possibly do that?

class PostImage(models.Model):
    media = GenericRelation('Media.Media')
    resource = models.OneToOneField('Post', null=True, related_name = 'image')

This already guaranteed you that there can only be a single Image holding media object attached to each post, by design.

Drop either of the relations. In a relational database, you want either a forward, or a reverse reference, but never both. Double linkage in a relational database is in fact not even possible once you start enforcing constraints. Not to mention that it is very easy to create an incoherent state, by inserting sets where the reversed relation points to the wrong row.

I get that you want to express ownership of Media in order to be able to collect orphans, but that's not the way to do that.

Let's just assume that double linkage itself isn't horribly bad, even then unique_together = ('content_type', 'object_id',) remains a bad idea. Why would you enforce an 1:0..1 in Media, when you only know that multiplicity constraint for Posts and Media?

Just drop that unique_together, and model the multiplicity solely via PostImage.

If you want to reliably clear out orphaned media, just write yourself a little garbage collector for that purpose, in that case you can even safely drop that GenericForeignKey all together. Finding media which isn't referenced by any other model should be quite easy.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thank you for your well thought out answer. I've gotten some help outside of this question and I've now done it like this: Instead of using different models to relate a media object to a post, I'm now storing the media directly on the post as "media" as a generic relation. What I am also doing is storing a "media_type" on the media object (so that you can have different types of media, for example a post might have a background image as well as an image) Now all I have to do to fetch some media is for example Post.media.get(media_type="background_image").images.all() \$\endgroup\$ Commented Oct 28, 2016 at 17:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.