From 9658de72a2c693505dbe8bae3501a25b934d9aa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laura=20Kl=C3=BCnder?= Date: Tue, 11 Oct 2016 16:33:12 +0200 Subject: [PATCH] use numeric primary keys, but still use unique names as lookups in the API --- src/c3nav/api/serializers.py | 10 +++--- src/c3nav/editor/api.py | 4 +++ src/c3nav/editor/hosters/base.py | 4 --- src/c3nav/editor/serializers.py | 4 +-- src/c3nav/mapdata/api.py | 6 ++++ src/c3nav/mapdata/migrations/0001_initial.py | 30 +++++++---------- .../migrations/0002_auto_20160926_0858.py | 33 ------------------- .../migrations/0003_auto_20160929_0736.py | 20 ----------- src/c3nav/mapdata/models/feature.py | 6 +--- src/c3nav/mapdata/models/level.py | 2 +- src/c3nav/mapdata/models/package.py | 2 +- src/c3nav/mapdata/models/source.py | 2 +- src/c3nav/mapdata/packageio/read.py | 9 +++++ src/c3nav/mapdata/serializers.py | 9 ++--- 14 files changed, 46 insertions(+), 95 deletions(-) delete mode 100644 src/c3nav/mapdata/migrations/0002_auto_20160926_0858.py delete mode 100644 src/c3nav/mapdata/migrations/0003_auto_20160929_0736.py diff --git a/src/c3nav/api/serializers.py b/src/c3nav/api/serializers.py index 59b0965b..54640ad2 100644 --- a/src/c3nav/api/serializers.py +++ b/src/c3nav/api/serializers.py @@ -4,13 +4,13 @@ from django.db.models.manager import BaseManager from rest_framework import serializers -class PkField(serializers.DictField): +class RelatedNameField(serializers.DictField): """ give primary key """ def to_representation(self, obj): - if hasattr(obj, 'pk'): - return obj.pk + if hasattr(obj, 'name'): + return obj.name elif isinstance(obj, Iterable): return tuple(self.to_representation(elem) for elem in obj) elif isinstance(obj, BaseManager): @@ -30,7 +30,7 @@ class RecursiveSerializerMixin(serializers.Serializer): for name in getattr(self.Meta, 'sparse_exclude', ()): value = self.fields.get(name) if value is not None and isinstance(value, serializers.Serializer): - self.fields[name] = PkField() + self.fields[name] = RelatedNameField() if request_sparse: for name in tuple(self.fields): @@ -42,5 +42,5 @@ class RecursiveSerializerMixin(serializers.Serializer): def recursive_value(self, serializer, obj, *args, **kwargs): if self.context.get('sparse'): - return PkField().to_representation(obj) + return RelatedNameField().to_representation(obj) return serializer(obj, context=self.sparse_context(), *args, **kwargs).data diff --git a/src/c3nav/editor/api.py b/src/c3nav/editor/api.py index cdea8099..eeecc543 100644 --- a/src/c3nav/editor/api.py +++ b/src/c3nav/editor/api.py @@ -18,6 +18,8 @@ class HosterViewSet(ViewSet): """ Retrieve and interact with package hosters """ + lookup_field = 'name' + def retrieve(self, request, pk=None): if pk not in hosters: raise Http404 @@ -92,6 +94,8 @@ class SubmitTaskViewSet(ViewSet): """ Get hoster submit tasks """ + lookup_field = 'id' + def retrieve(self, request, pk=None): task = submit_edit_task.AsyncResult(task_id=pk) try: diff --git a/src/c3nav/editor/hosters/base.py b/src/c3nav/editor/hosters/base.py index 3efc8bcc..600fd486 100644 --- a/src/c3nav/editor/hosters/base.py +++ b/src/c3nav/editor/hosters/base.py @@ -15,10 +15,6 @@ class Hoster(ABC): self.name = name self.base_url = base_url - @property - def pk(self): - return self.name - def get_packages(self): """ Get a Queryset of all packages that can be handled by this hoster diff --git a/src/c3nav/editor/serializers.py b/src/c3nav/editor/serializers.py index 91c9deb2..b9ed3241 100644 --- a/src/c3nav/editor/serializers.py +++ b/src/c3nav/editor/serializers.py @@ -4,7 +4,7 @@ from rest_framework.reverse import reverse class HosterSerializer(serializers.Serializer): name = serializers.CharField() - url = serializers.HyperlinkedIdentityField(view_name='api:hoster-detail') + url = serializers.HyperlinkedIdentityField(view_name='api:hoster-detail', lookup_field='name') state_url = serializers.SerializerMethodField() auth_uri_url = serializers.SerializerMethodField() submit_url = serializers.SerializerMethodField() @@ -22,7 +22,7 @@ class HosterSerializer(serializers.Serializer): class TaskSerializer(serializers.Serializer): id = serializers.CharField() - url = serializers.HyperlinkedIdentityField(view_name='api:hoster-detail', lookup_field='id', lookup_url_kwarg='pk') + url = serializers.HyperlinkedIdentityField(view_name='api:hoster-detail', lookup_field='id') started = serializers.SerializerMethodField() done = serializers.SerializerMethodField() success = serializers.SerializerMethodField() diff --git a/src/c3nav/mapdata/api.py b/src/c3nav/mapdata/api.py index ae25b478..75354d54 100644 --- a/src/c3nav/mapdata/api.py +++ b/src/c3nav/mapdata/api.py @@ -20,6 +20,7 @@ class LevelViewSet(ReadOnlyModelViewSet): """ queryset = Level.objects.all() serializer_class = LevelSerializer + lookup_field = 'name' lookup_value_regex = '[^/]+' filter_fields = ('altitude', 'package') ordering_fields = ('altitude', 'package') @@ -33,6 +34,7 @@ class PackageViewSet(ReadOnlyModelViewSet): """ queryset = Package.objects.all() serializer_class = PackageSerializer + lookup_field = 'name' lookup_value_regex = '[^/]+' filter_fields = ('name', 'depends') ordering_fields = ('name',) @@ -46,6 +48,7 @@ class SourceViewSet(ReadOnlyModelViewSet): """ queryset = Source.objects.all() serializer_class = SourceSerializer + lookup_field = 'name' lookup_value_regex = '[^/]+' filter_fields = ('package',) ordering_fields = ('name', 'package') @@ -69,6 +72,8 @@ class FeatureTypeViewSet(ViewSet): """ List and retrieve feature types """ + lookup_field = 'name' + def list(self, request): serializer = FeatureTypeSerializer(FEATURE_TYPES.values(), many=True, context={'request': request}) return Response(serializer.data) @@ -86,4 +91,5 @@ class FeatureViewSet(ReadOnlyModelViewSet): """ queryset = Feature.objects.all() serializer_class = FeatureSerializer + lookup_field = 'name' lookup_value_regex = '[^/]+' diff --git a/src/c3nav/mapdata/migrations/0001_initial.py b/src/c3nav/mapdata/migrations/0001_initial.py index 8182587c..b427d5e4 100644 --- a/src/c3nav/mapdata/migrations/0001_initial.py +++ b/src/c3nav/mapdata/migrations/0001_initial.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -# Generated by Django 1.10.1 on 2016-09-23 15:15 +# Generated by Django 1.10.1 on 2016-10-11 14:00 from __future__ import unicode_literals import c3nav.mapdata.fields @@ -17,32 +17,27 @@ class Migration(migrations.Migration): operations = [ migrations.CreateModel( name='Feature', - fields=[ - ('name', models.SlugField(help_text='e.g. noc', primary_key=True, serialize=False, verbose_name='feature identifier')), - ('feature_type', models.CharField(choices=[('building', 'Building'), ('room', 'Room'), ('outside', 'Outside Area'), ('obstacle', 'Obstacle')], max_length=50)), - ('geometry', c3nav.mapdata.fields.GeometryField()), - ], - ), - migrations.CreateModel( - name='FeatureTitle', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('language', models.CharField(max_length=50)), - ('title', models.CharField(max_length=50)), - ('feature', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='featuretitles', to='mapdata.Feature', verbose_name='map package')), + ('name', models.SlugField(unique=True, verbose_name='feature identifier')), + ('feature_type', models.CharField(choices=[('building', 'Building'), ('room', 'Room'), ('outside', 'Outside Area'), ('obstacle', 'Obstacle')], max_length=50)), + ('titles', c3nav.mapdata.fields.JSONField()), + ('geometry', c3nav.mapdata.fields.GeometryField()), ], ), migrations.CreateModel( name='Level', fields=[ - ('name', models.SlugField(help_text='Usually just an integer (e.g. -1, 0, 1, 2)', primary_key=True, serialize=False, verbose_name='level name')), + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.SlugField(help_text='Usually just an integer (e.g. -1, 0, 1, 2)', unique=True, verbose_name='level name')), ('altitude', models.DecimalField(decimal_places=2, max_digits=6, null=True, verbose_name='level altitude')), ], ), migrations.CreateModel( name='Package', fields=[ - ('name', models.SlugField(help_text='e.g. de.c3nav.33c3.base', primary_key=True, serialize=False, verbose_name='package identifier')), + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.SlugField(help_text='e.g. de.c3nav.33c3.base', unique=True, verbose_name='package identifier')), ('home_repo', models.URLField(null=True, verbose_name='URL to the home git repository')), ('commit_id', models.CharField(max_length=40, null=True, verbose_name='current commit id')), ('bottom', models.DecimalField(decimal_places=2, max_digits=6, null=True, verbose_name='bottom coordinate')), @@ -56,7 +51,8 @@ class Migration(migrations.Migration): migrations.CreateModel( name='Source', fields=[ - ('name', models.SlugField(primary_key=True, serialize=False, verbose_name='source name')), + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.SlugField(unique=True, verbose_name='source name')), ('bottom', models.DecimalField(decimal_places=2, max_digits=6, verbose_name='bottom coordinate')), ('left', models.DecimalField(decimal_places=2, max_digits=6, verbose_name='left coordinate')), ('top', models.DecimalField(decimal_places=2, max_digits=6, verbose_name='top coordinate')), @@ -79,8 +75,4 @@ class Migration(migrations.Migration): name='package', field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='features', to='mapdata.Package', verbose_name='map package'), ), - migrations.AlterUniqueTogether( - name='featuretitle', - unique_together=set([('feature', 'language')]), - ), ] diff --git a/src/c3nav/mapdata/migrations/0002_auto_20160926_0858.py b/src/c3nav/mapdata/migrations/0002_auto_20160926_0858.py deleted file mode 100644 index 64849cb5..00000000 --- a/src/c3nav/mapdata/migrations/0002_auto_20160926_0858.py +++ /dev/null @@ -1,33 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.10.1 on 2016-09-26 08:58 -from __future__ import unicode_literals - -import c3nav.mapdata.fields -from django.db import migrations - - -class Migration(migrations.Migration): - - dependencies = [ - ('mapdata', '0001_initial'), - ] - - operations = [ - migrations.AlterUniqueTogether( - name='featuretitle', - unique_together=set([]), - ), - migrations.RemoveField( - model_name='featuretitle', - name='feature', - ), - migrations.AddField( - model_name='feature', - name='titles', - field=c3nav.mapdata.fields.JSONField(default={}), - preserve_default=False, - ), - migrations.DeleteModel( - name='FeatureTitle', - ), - ] diff --git a/src/c3nav/mapdata/migrations/0003_auto_20160929_0736.py b/src/c3nav/mapdata/migrations/0003_auto_20160929_0736.py deleted file mode 100644 index bfebb538..00000000 --- a/src/c3nav/mapdata/migrations/0003_auto_20160929_0736.py +++ /dev/null @@ -1,20 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.10.1 on 2016-09-29 07:36 -from __future__ import unicode_literals - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('mapdata', '0002_auto_20160926_0858'), - ] - - operations = [ - migrations.AlterField( - model_name='feature', - name='name', - field=models.SlugField(primary_key=True, serialize=False, verbose_name='feature identifier'), - ), - ] diff --git a/src/c3nav/mapdata/models/feature.py b/src/c3nav/mapdata/models/feature.py index c41e907c..5586596d 100644 --- a/src/c3nav/mapdata/models/feature.py +++ b/src/c3nav/mapdata/models/feature.py @@ -16,10 +16,6 @@ class FeatureType(namedtuple('FeatureType', ('name', 'title', 'title_plural', 'g super().__init__() FEATURE_TYPES[self.name] = self - @property - def pk(self): - return self.name - @property def title_en(self): language = get_language() @@ -47,7 +43,7 @@ class Feature(models.Model): """ TYPES = tuple((name, t.title) for name, t in FEATURE_TYPES.items()) - name = models.SlugField(_('feature identifier'), primary_key=True, max_length=50) + name = models.SlugField(_('feature identifier'), unique=True, max_length=50) package = models.ForeignKey('mapdata.Package', on_delete=models.CASCADE, related_name='features', verbose_name=_('map package')) feature_type = models.CharField(max_length=50, choices=TYPES) diff --git a/src/c3nav/mapdata/models/level.py b/src/c3nav/mapdata/models/level.py index d27756fe..4c817bd1 100644 --- a/src/c3nav/mapdata/models/level.py +++ b/src/c3nav/mapdata/models/level.py @@ -6,7 +6,7 @@ class Level(models.Model): """ A map level (-1, 0, 1, 2…) """ - name = models.SlugField(_('level name'), primary_key=True, max_length=50, + name = models.SlugField(_('level name'), unique=True, max_length=50, help_text=_('Usually just an integer (e.g. -1, 0, 1, 2)')) altitude = models.DecimalField(_('level altitude'), null=True, max_digits=6, decimal_places=2) package = models.ForeignKey('mapdata.Package', on_delete=models.CASCADE, related_name='levels', diff --git a/src/c3nav/mapdata/models/package.py b/src/c3nav/mapdata/models/package.py index b9dcfc90..b9aee494 100644 --- a/src/c3nav/mapdata/models/package.py +++ b/src/c3nav/mapdata/models/package.py @@ -9,7 +9,7 @@ class Package(models.Model): """ A c3nav map package """ - name = models.SlugField(_('package identifier'), primary_key=True, max_length=50, + name = models.SlugField(_('package identifier'), unique=True, max_length=50, help_text=_('e.g. de.c3nav.33c3.base')) depends = models.ManyToManyField('Package') home_repo = models.URLField(_('URL to the home git repository'), null=True) diff --git a/src/c3nav/mapdata/models/source.py b/src/c3nav/mapdata/models/source.py index c9c10919..ded8e93e 100644 --- a/src/c3nav/mapdata/models/source.py +++ b/src/c3nav/mapdata/models/source.py @@ -6,7 +6,7 @@ class Source(models.Model): """ A map source, images of levels that can be useful as backgrounds for the map editor """ - name = models.SlugField(_('source name'), primary_key=True, max_length=50) + name = models.SlugField(_('source name'), unique=True, max_length=50) package = models.ForeignKey('mapdata.Package', on_delete=models.CASCADE, related_name='sources', verbose_name=_('map package')) diff --git a/src/c3nav/mapdata/packageio/read.py b/src/c3nav/mapdata/packageio/read.py index b1319074..79b2944d 100644 --- a/src/c3nav/mapdata/packageio/read.py +++ b/src/c3nav/mapdata/packageio/read.py @@ -146,9 +146,13 @@ class ReaderItem: } def save(self): + depends = [] if self.model != Package: package_name = self.reader.package_names_by_dir[self.package_dir] self.data['package'] = self.reader.saved_items[Package][package_name].obj + else: + depends = [self.reader.saved_items[Package][name].obj.pk for name in self.data['depends']] + self.data.pop('depends') # Change name references to the referenced object for name, model in self.relations.items(): @@ -161,3 +165,8 @@ class ReaderItem: self.obj = obj self.reader.saved_items[self.model][obj.name] = self + + if depends: + self.obj.depends.clear() + for dependency in depends: + self.obj.depends.add(dependency) diff --git a/src/c3nav/mapdata/serializers.py b/src/c3nav/mapdata/serializers.py index 1c3e8edf..90613f25 100644 --- a/src/c3nav/mapdata/serializers.py +++ b/src/c3nav/mapdata/serializers.py @@ -40,7 +40,7 @@ class PackageSerializer(RecursiveSerializerMixin, serializers.ModelSerializer): fields = ('name', 'url', 'home_repo', 'commit_id', 'depends', 'bounds', 'public', 'hoster') sparse_exclude = ('depends', 'hoster') extra_kwargs = { - 'url': {'view_name': 'api:package-detail'} + 'url': {'view_name': 'api:package-detail', 'lookup_field': 'name'} } def get_depends(self, obj): @@ -59,7 +59,7 @@ class LevelSerializer(RecursiveSerializerMixin, serializers.ModelSerializer): fields = ('name', 'url', 'altitude', 'package') sparse_exclude = ('package',) extra_kwargs = { - 'url': {'view_name': 'api:level-detail'} + 'url': {'view_name': 'api:level-detail', 'lookup_field': 'name'} } @@ -72,7 +72,7 @@ class SourceSerializer(RecursiveSerializerMixin, serializers.ModelSerializer): fields = ('name', 'url', 'image_url', 'package', 'bounds') sparse_exclude = ('package', ) extra_kwargs = { - 'url': {'view_name': 'api:source-detail'} + 'url': {'view_name': 'api:source-detail', 'lookup_field': 'name'} } def get_image_url(self, obj): @@ -100,7 +100,8 @@ class FeatureSerializer(RecursiveSerializerMixin, serializers.ModelSerializer): fields = ('name', 'url', 'title', 'feature_type', 'level', 'titles', 'package', 'geometry') sparse_exclude = ('feature_type', 'level', 'package') extra_kwargs = { - 'url': {'view_name': 'api:feature-detail'} + 'lookup_field': 'name', + 'url': {'view_name': 'api:feature-detail', 'lookup_field': 'name'} } def get_feature_type(self, obj):