stash

backend/toolshed/api/inventory.py

@@ -149,6 +149,10 @@ class StorageLocationViewSet(viewsets.ModelViewSet):
     serializer_class = StorageLocationSerializer
     authentication_classes = [SignatureAuthentication]
     permission_classes = [IsAuthenticated]
+    # Detail routes address a location by its owner-scoped id, not the internal row id. See
+    # docs/implementation.md#inventory-detail-routes-use-owner-scoped-ids.
+    lookup_field = 'id'
+    lookup_url_kwarg = 'pk'
 
     def get_queryset(self):
         if type(self.request.user) == KnownIdentity and self.request.user.user.exists():

backend/toolshed/models.py

@@ -169,7 +169,27 @@ class ItemTag(models.Model):
     inventory_item = models.ForeignKey(InventoryItem, on_delete=models.CASCADE)
 
 
+class OwnerStorageLocationSequence(models.Model):
+    """Tracks the last StorageLocation id handed out per owner for sequential, gapless allocation
+    (see StorageLocation.create_for_owner)."""
+    owner = models.ForeignKey(ToolshedUser, on_delete=models.CASCADE, related_name='+', unique=True)
+    last_id = models.PositiveIntegerField(default=0)
+
+    @classmethod
+    def allocate(cls, *, owner):
+        with transaction.atomic():
+            seq, _ = cls.objects.select_for_update().get_or_create(owner=owner)
+            seq.last_id += 1
+            seq.save(update_fields=['last_id'])
+            return seq.last_id
+
+
 class StorageLocation(models.Model):
+    internal_id = models.AutoField(primary_key=True)
+    # Externally visible id, sequential/gapless within the owner's own locations (see
+    # OwnerStorageLocationSequence), never internal_id; always allocate via create_for_owner, not
+    # .objects.create().
+    id = models.PositiveIntegerField(editable=False)
     name = models.CharField(max_length=255)
     description = models.TextField(null=True, blank=True)
     category = models.ForeignKey(Category, on_delete=models.CASCADE, null=True, blank=True,
@@ -177,10 +197,23 @@ class StorageLocation(models.Model):
     parent = models.ForeignKey('self', on_delete=models.CASCADE, null=True, blank=True, related_name='children')
     owner = models.ForeignKey(ToolshedUser, on_delete=models.CASCADE, related_name='storage_locations')
 
+    class Meta:
+        constraints = [
+            models.UniqueConstraint(fields=['owner', 'id'], name='storagelocation_unique_owner_scoped_id'),
+        ]
+
     def __str__(self):
         parent = str(self.parent) + "/" if self.parent else ""
         return parent + self.name
 
+    @classmethod
+    def create_for_owner(cls, *, owner, **kwargs):
+        """The only supported way to create a StorageLocation: atomically allocates the next id
+        for this owner's scope."""
+        with transaction.atomic():
+            next_id = OwnerStorageLocationSequence.allocate(owner=owner)
+            return cls.objects.create(owner=owner, id=next_id, **kwargs)
+
 
 class WorkflowInstance(models.Model):
     slug = models.CharField(max_length=255)

backend/toolshed/offlinedata.py

@@ -339,13 +339,17 @@ def import_locations(user, data):
                 if category_path:
                     category = get_or_create_category(category_path)
 
-                location, _ = StorageLocation.objects.update_or_create(
-                    owner=user, name=name, parent=parent,
-                    defaults={
-                        'description': row.get('description', '') or '',
-                        'category': category,
-                    },
-                )
+                defaults = {
+                    'description': row.get('description', '') or '',
+                    'category': category,
+                }
+                try:
+                    location = StorageLocation.objects.get(owner=user, name=name, parent=parent)
+                    for field, value in defaults.items():
+                        setattr(location, field, value)
+                    location.save(update_fields=list(defaults.keys()))
+                except StorageLocation.DoesNotExist:
+                    location = StorageLocation.create_for_owner(owner=user, name=name, parent=parent, **defaults)
                 resolved_by_path[path] = location
                 imported += 1
         except Exception as error:

backend/toolshed/serializers.py

@@ -1,3 +1,4 @@
+from django.core.exceptions import ObjectDoesNotExist
 from rest_framework import serializers
 from authentication.models import KnownIdentity, ToolshedUser, FriendRequestIncoming, Group, GroupInviteIncoming
 from authentication.serializers import OwnerSerializer, GroupOwnerSerializer
@@ -150,15 +151,50 @@ class CategorySerializer(serializers.ModelSerializer):
         return resolve_category_handle(data.split("/")[-1])
 
 
+class OwnerScopedPrimaryKeyRelatedField(serializers.PrimaryKeyRelatedField):
+    """Resolves/represents by the owner-scoped `id` rather than the model's internal pk, scoped to
+    the requesting user - StorageLocation.parent points at another StorageLocation, whose publicly
+    visible identity is now the owner-scoped id (see StorageLocation.create_for_owner), not
+    internal_id."""
+
+    def use_pk_only_optimization(self):
+        # False: to_representation needs the owner-scoped `id`, not just the internal pk that the
+        # PKOnlyObject optimization would otherwise limit us to.
+        return False
+
+    def get_queryset(self):
+        queryset = super().get_queryset()
+        request = self.context.get('request')
+        if request is not None and type(request.user) == KnownIdentity and request.user.user.exists():
+            return queryset.filter(owner=request.user.user.get())
+        return queryset.none()
+
+    def to_internal_value(self, data):
+        queryset = self.get_queryset()
+        try:
+            if isinstance(data, bool):
+                raise TypeError
+            return queryset.get(id=data)
+        except ObjectDoesNotExist:
+            self.fail('does_not_exist', pk_value=data)
+        except (TypeError, ValueError):
+            self.fail('incorrect_type', data_type=type(data).__name__)
+
+    def to_representation(self, value):
+        return value.id
+
+
 class StorageLocationSerializer(serializers.ModelSerializer):
     owner = OwnerSerializer(read_only=True)
     category = serializers.CharField(required=False, allow_null=True, allow_blank=True)
+    parent = OwnerScopedPrimaryKeyRelatedField(queryset=StorageLocation.objects.all(), required=False,
+                                               allow_null=True)
     path = serializers.SerializerMethodField()
 
     class Meta:
         model = StorageLocation
         fields = ['id', 'name', 'description', 'path', 'category', 'owner', 'parent']
-        read_only_fields = ['path']
+        read_only_fields = ['id', 'path']
 
     @staticmethod
     def get_path(obj):
@@ -166,6 +202,9 @@ class StorageLocationSerializer(serializers.ModelSerializer):
             return StorageLocationSerializer.get_path(obj.parent) + "/" + obj.name
         return obj.name
 
+    def create(self, validated_data):
+        return StorageLocation.create_for_owner(**validated_data)
+
 
 class ItemPropertySerializer(serializers.ModelSerializer):
     property = PropertySerializer(read_only=True)
@@ -195,6 +234,8 @@ class InventoryItemSerializer(serializers.ModelSerializer):
     properties = ItemPropertySerializer(many=True, required=False, source='itemproperty_set')
     category = CategorySerializer(required=False, allow_null=True)
     files = FileSerializer(many=True, read_only=True)
+    storage_location = OwnerScopedPrimaryKeyRelatedField(queryset=StorageLocation.objects.all(), required=False,
+                                                          allow_null=True)
 
     class Meta:
         model = InventoryItem

backend/toolshed/tests/fixtures.py

@@ -49,12 +49,12 @@ class InventoryTestMixin(CategoryTestMixin, TagTestMixin, PropertyTestMixin):
 
 class LocationTestMixin:
     def prepare_locations(self):
-        self.f['loc1'] = StorageLocation.objects.create(name='loc1', owner=self.f['local_user1'])
-        self.f['loc2'] = StorageLocation.objects.create(name='loc2', owner=self.f['local_user1'],
-                                                        category=self.f['cat1'])
-        self.f['loc3'] = StorageLocation.objects.create(name='loc3', owner=self.f['local_user1'], parent=self.f['loc1'])
-        self.f['loc4'] = StorageLocation.objects.create(name='loc4', owner=self.f['local_user1'], parent=self.f['loc1'],
-                                                        category=self.f['cat1'])
+        self.f['loc1'] = StorageLocation.create_for_owner(name='loc1', owner=self.f['local_user1'])
+        self.f['loc2'] = StorageLocation.create_for_owner(name='loc2', owner=self.f['local_user1'],
+                                                          category=self.f['cat1'])
+        self.f['loc3'] = StorageLocation.create_for_owner(name='loc3', owner=self.f['local_user1'], parent=self.f['loc1'])
+        self.f['loc4'] = StorageLocation.create_for_owner(name='loc4', owner=self.f['local_user1'], parent=self.f['loc1'],
+                                                          category=self.f['cat1'])
 
 
 class WorkflowTestMixin: