3
\$\begingroup\$

For flexing my newly acquired Django & Python muscles, I have set up a mobile content delivery system via Wap Push (Ringtones, wallpapers, etc).

The idea is that a keyword comes in from an sms via an URL, let's say the keyword is "LOVE1" and the program should search if this keyboard points to a Ringtone or an Image. For this I have created a parent model class called "Categoria" (Category) and two subclasses "Ringtone" and "Wallpaper". This subclasses have a variable called "archivo" (filename) which points to the actual path of the content.

Dynpath is a dynamic URL which has been created to download the content, so it is available only for X amount of time. After that a Celery scheduled task deletes this dynamic URL from the DB.

I have a piece which has "code smell" which I would like to have some input from everyone here.

Model

class Contenido(models.Model):
    nombre = models.CharField(max_length=100)
    fecha_creacion = models.DateTimeField('fecha creacion')
    keyword = models.CharField(max_length=100)

class Ringtone(Contenido):
    grupo = models.ManyToManyField(Artista)
    archivo = models.FileField(upload_to="uploads")

    def __unicode__(self):
        return self.nombre

class Wallpaper(Contenido):
    categoria = models.ForeignKey(Categoria)
    archivo = models.ImageField(upload_to="uploads")

    def __unicode__(self):
        return self.nombre

class Dynpath(models.Model):
    created = models.DateField(auto_now=True)
    url_path = models.CharField(max_length=100)
    payload = models.ForeignKey(Contenido)
    sms = models.ForeignKey(SMS)

    def __unicode__(self):
        return str(self.url_path)

View

Here is my view which checks that the Dynamic URL exists and here is where the code (which works) gets a little suspicious/ugly:

    def tempurl(request,hash):
        p = get_object_or_404(Dynpath, url_path=hash)
        try:
            fname = str(p.payload.wallpaper.archivo)
        except DoesNotExist:
            fname = str(p.payload.ringtone.archivo)

        fn = open(fname,'rb')
        response = HttpResponse(fn.read())
        fn.close()
        file_name = os.path.basename(fname)
        type, encoding = mimetypes.guess_type(file_name)
        if type is None:
            type = 'application/octet-stream'
        response['Content-Type'] = type
        response['Content-Disposition'] = ('attachment; filename=%s') % file_name
        return response

I am talking explictitly this snippet:

      try:
            fname = str(p.payload.wallpaper.archivo)
        except DoesNotExist:
            fname = str(p.payload.ringtone.archivo)

I would have loved to do something like:

fname = p.payload.archivo

But it would not let me do that, from the docs:

Django will raise a FieldError if you override any model field in any ancestor model.

I took a look at generics, but could not make it work with them. Any ideas on a better way of doing this?

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

You have 2 models (Ringtone extends Contenido). As I understand you store same nombre, fecha_creacion, keyword in both models and every update/delete/insert operation on the first model must be synchronized with another one. You can avoid this, make foreign key to base model:

class Contenido(models.Model):
    nombre = models.CharField(max_length=100)
    fecha_creacion = models.DateTimeField('fecha creacion')
    keyword = models.CharField(max_length=100)

class Ringtone(models.Model):
    contenido = models.ForeignKey(Contenido)
    grupo = models.ManyToManyField(Artista)
    archivo = models.FileField(upload_to="uploads")

class Wallpaper(models.Model):
    contenido = models.ForeignKey(Contenido)    
    categoria = models.ForeignKey(Categoria)
    archivo = models.ImageField(upload_to="uploads")

Then in your Views

def tempurl(request,hash):
    p = get_object_or_404(Dynpath, url_path=hash)
    try:
        obj=Wallpaper.objects.get(contenido_id=p.id)
    except Wallpaper.DoesNotExist:
        try:
            obj=Ringtone.objects.get(contenido_id=p.id)
        except Ringtone.DoesNotExist:
            raise Http404 
    fname = str(obj.archivo)

    # use with statement
    with open(fname,'rb') as fn:
        response = HttpResponse(fn.read())

Uhh.. Is it still complicated? If you can retrieve content type (ringtone or wallpaper) and save it in Dynpath field, solution will be easier.

P.S. Please write your code in English not Spanish )

\$\endgroup\$
2
  • \$\begingroup\$ Thank you for review. I am not quite sure I am getting your point: As the relationship between Ringtone->Contenido is one to one, as is the case also for the relationship between Wallpaper->Contenido. and adding this foreign key would imply a many to one relationship. In other words, one keyword can only correspond to one file (archivo). My question was more about, what happens when I have not only Wallpaper and Ringtones.. lets say I must add a Game category, I would have to keep editing the code in tempurl view. Sorry about the spanish :( \$\endgroup\$
    – AlexR
    Mar 1, 2012 at 10:17
  • \$\begingroup\$ @AlexR Yes, you can use OneToOne relationship instead of OneToMany and it will be more strict and correct. But you inherited Ringtone and Wallpaper from base model Contenido, what is bad. In your case, if you want to add Game category you will have to fix template view. As I have written in the end of my answer, you can add content type to your Dynpath model and then calculate related model class Model. Then you will do obj=Model.objects.get(contenido_id=p.id) and get filename \$\endgroup\$
    – San4ez
    Mar 1, 2012 at 11:36

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.