From ed04d98bf18deace78aea36a518ed15875fc0c7d Mon Sep 17 00:00:00 2001 From: jedi Date: Mon, 24 Aug 2026 15:57:17 +0200 Subject: [PATCH] stash --- backend/authentication/api.py | 24 +- backend/authentication/models.py | 13 +- backend/authentication/signature_auth.py | 14 +- backend/backend/asgi.py | 9 - backend/backend/settings.py | 45 +-- backend/backend/urls.py | 15 - backend/backend/wsgi.py | 9 - backend/configure.py | 7 +- backend/files/media_urls.py | 50 +-- backend/files/models.py | 10 +- backend/files/tests.py | 28 +- backend/toolshed/api/files.py | 10 +- backend/toolshed/api/inventory.py | 27 +- backend/toolshed/models.py | 15 +- backend/toolshed/offlinedata.py | 131 +------- backend/toolshed/serializers.py | 6 +- backend/toolshed/tests/test_friend.py | 16 +- backend/toolshed/tests/test_inventory.py | 26 +- backend/toolshed/tests/test_offlinedata.py | 10 +- deploy/dev/Dockerfile.proxy | 13 - deploy/dev/docker-compose.yml | 10 + docs/index.md | 1 + frontend/src/cameraManager.js | 6 +- .../src/components/AuthenticatedImage.vue | 5 +- .../src/components/LabelLayoutPreview.vue | 94 ++++-- .../components/inputs/CameraFileSource.vue | 4 +- .../components/inputs/DragDropFileSource.vue | 4 +- .../src/components/inputs/FsFileSource.vue | 4 +- .../components/inputs/WebcamFileSource.vue | 4 +- .../workflows/BulkItemImportWorkflow.vue | 31 +- .../workflows/FotoFirstBulkImportWorkflow.vue | 26 +- .../FotoFirstBulkImportWorkflow2.vue | 15 +- frontend/src/federation.js | 8 +- frontend/src/fileCache.js | 16 +- frontend/src/handle-url.js | 8 +- frontend/src/label-layouts.js | 74 ++--- frontend/src/label.js | 311 +++++------------- frontend/src/router.js | 26 +- frontend/src/scss/_pixel-fonts.scss | 13 +- frontend/src/scss/toolshed.scss | 1 + frontend/src/store.js | 63 ++-- frontend/src/tests/short-id.js | 4 +- frontend/src/views/Inventory.vue | 14 +- frontend/src/views/InventoryDetail.vue | 9 +- frontend/src/views/InventoryEdit.vue | 6 +- frontend/src/views/Print.vue | 256 +++++--------- frontend/src/views/Scan.vue | 266 ++++++++++++--- frontend/src/views/Search.vue | 6 +- frontend/src/views/ShortId.vue | 10 +- frontend/src/views/StorageLocation.vue | 7 +- frontend/src/views/Swatch.vue | 27 +- frontend/src/views/WorkflowDetail.vue | 4 +- frontend/src/views/Workflows.vue | 4 - frontend/src/workflows.js | 60 +--- 54 files changed, 661 insertions(+), 1214 deletions(-) diff --git a/backend/authentication/api.py b/backend/authentication/api.py index 1e3f09c..599fea5 100644 --- a/backend/authentication/api.py +++ b/backend/authentication/api.py @@ -18,9 +18,8 @@ from hostadmin.models import Domain router = routers.SimpleRouter() -# Schema for the account-level preferences a client may store on the server (see -# AccountPreference). Device-level preferences are never sent here - they stay in the -# browser's local storage since they describe the device, not the account. +# Schema for account-level preferences a client may store server-side (see AccountPreference); +# device-level preferences stay in the browser's local storage instead. PREFERENCE_DEFINITIONS = [ { 'key': 'ui.compact_mode', @@ -102,9 +101,8 @@ class UserViewSet(viewsets.ModelViewSet): @permission_classes([IsAuthenticated]) @authentication_classes([SignatureAuthenticationLocal]) def getUserInfo(request): - """Get or update the authenticated local user's own account info. Only usable by the - account owner on their own home server - see getUserProfile for viewing another (friend) - user's public profile.""" + """Get or update the authenticated local user's own account info; only the account owner may + call this on their own home server (see getUserProfile for viewing a friend's public profile).""" user = request.user if request.method == 'PATCH': old_file = user.profile_picture @@ -147,9 +145,9 @@ def getUserInfo(request): @permission_classes([IsAuthenticated]) @authentication_classes([SignatureAuthentication]) def getUserProfile(request, handle): - """Get another local user's public profile by handle (username@domain), e.g. so a friend - can look up someone's avatar. The caller must be a friend of that user (or the user - itself, signing with their own known identity rather than their local credentials).""" + """Get another local user's public profile by handle, e.g. so a friend can look up an avatar; + caller must be a friend of that user (or the user itself, signing with their own known + identity rather than local credentials).""" try: username, domain = split_userhandle_or_throw(handle) except ValueError: @@ -213,11 +211,9 @@ def preference_definitions(request): @permission_classes([IsAuthenticated]) @authentication_classes([SignatureAuthenticationLocal]) def account_preferences(request): - """Get or bulk-upsert the authenticated user's account-level preferences. - - GET returns the current preferences as a {key: value} dict. PUT accepts a {key: value} - dict of one or more preferences to set/overwrite; unspecified keys are left untouched. - """ + """Get or bulk-upsert the authenticated user's account-level preferences: GET returns the + current preferences as {key: value}; PUT sets/overwrites one or more, leaving unspecified + keys untouched.""" if request.method == 'PUT': if not isinstance(request.data, dict): return Response({'detail': 'Expected an object of key/value pairs.'}, status=400) diff --git a/backend/authentication/models.py b/backend/authentication/models.py index e120ce8..a19e759 100644 --- a/backend/authentication/models.py +++ b/backend/authentication/models.py @@ -114,11 +114,8 @@ class ToolshedUser(AbstractUser): class AccountPreference(models.Model): - """A single account-level (server-synced, cross-device) user preference, stored as a key/value pair. - - Device-level preferences are intentionally *not* stored here - they stay in the browser's - local storage since they describe the device, not the account. - """ + """A single account-level (server-synced, cross-device) preference as a key/value pair; + device-level preferences are intentionally *not* stored here.""" user = models.ForeignKey(ToolshedUser, on_delete=models.CASCADE, related_name='preferences') key = models.CharField(max_length=255) value = models.JSONField() @@ -165,9 +162,9 @@ class Group(models.Model): class GroupInvite(models.Model): - """A pending invite tracked on the group's own home backend, checked when the invitee's - accept request arrives (see GroupInviteIncoming for the mirror record on the invitee's own - backend, and docs/design-in-progress/groups-mvp.md for the full invite/accept dance).""" + """A pending invite tracked on the group's own home backend, checked when the invitee's accept + request arrives (mirror: GroupInviteIncoming on the invitee's backend; see + docs/design-in-progress/groups-mvp.md).""" secret = models.CharField(max_length=255) group = models.ForeignKey(Group, on_delete=models.CASCADE, related_name='invites') invitee_username = models.CharField(max_length=255) diff --git a/backend/authentication/signature_auth.py b/backend/authentication/signature_auth.py index 2be90d3..1d2d247 100644 --- a/backend/authentication/signature_auth.py +++ b/backend/authentication/signature_auth.py @@ -81,12 +81,8 @@ def verify_incoming_friend_request(request, raw_request_body): def verify_incoming_group_invite(request, raw_request_body, handle_field, key_field): - """Self-certifying verifier for the two legs of the group invite/accept dance that land on a - backend which doesn't have the caller cached as a KnownIdentity yet (see - docs/design-in-progress/groups-mvp.md): the inviter delivering an invite to the invitee's own - backend (handle_field='inviter', key_field='inviter_key'), and the invitee accepting on the - group's home backend (handle_field='invitee', key_field='invitee_key'). Mirrors - verify_incoming_friend_request exactly, just with configurable field names.""" + """Self-certifying verifier for the group invite/accept dance. See + docs/implementation.md#group-invite-and-accept-self-certifying-verification.""" try: username, domain, signed_data, signature_bytes_hex = verify_request(request, raw_request_body) except ValueError: @@ -143,10 +139,8 @@ def authenticate_request_against_local_users(request, raw_request_body): class SignatureAuthentication(authentication.BaseAuthentication): def authenticate(self, request): identity = authenticate_request_against_known_identities(request, request.body.decode('utf-8')) - # Returning a bare None (rather than a (None, None) tuple) tells DRF this - # authenticator doesn't apply, so it moves on to the next authenticator in the - # authentication_classes list instead of treating the request as authenticated - # with an empty user. + # Bare None (not a (None, None) tuple) tells DRF to try the next authenticator, instead + # of treating the request as authenticated with an empty user. if identity is None: return None return identity, None diff --git a/backend/backend/asgi.py b/backend/backend/asgi.py index f4518a1..c6fe45e 100644 --- a/backend/backend/asgi.py +++ b/backend/backend/asgi.py @@ -1,12 +1,3 @@ -""" -ASGI config for backend project. - -It exposes the ASGI callable as a module-level variable named ``application``. - -For more information on this file, see -https://docs.djangoproject.com/en/4.1/howto/deployment/asgi/ -""" - import os from django.core.asgi import get_asgi_application diff --git a/backend/backend/settings.py b/backend/backend/settings.py index e56e4ab..db7893c 100644 --- a/backend/backend/settings.py +++ b/backend/backend/settings.py @@ -1,14 +1,3 @@ -""" -Django settings for backend project. - -Generated by 'django-admin startproject' using Django 4.2.2. - -For more information on this file, see -https://docs.djangoproject.com/en/4.1/topics/settings/ - -For the full list of settings and their values, see -https://docs.djangoproject.com/en/4.1/ref/settings/ -""" import os import subprocess import dotenv @@ -19,13 +8,9 @@ BASE_DIR = Path(__file__).resolve().parent.parent def _git_commit(): - # deploy/dev/docker-compose.yml bind-mounts the repo's real .git dir at - # /git (separate from BASE_DIR, which only has the backend/ subtree) - - # point git at it explicitly there. Bare-metal dev has no such mount, but - # BASE_DIR sits inside the real checkout so plain rev-parse finds it by - # walking up. In prod, the build context is backend/ alone with no .git - # anywhere, so both fail and we fall back to the GIT_COMMIT build-arg/env - # var (see deploy/prod/Dockerfile.backend and playbook.yml). + # Docker dev bind-mounts the real .git dir at /git (see docker-compose.yml); bare-metal dev + # finds it by walking up from BASE_DIR instead. Prod has no .git at all, so both fail and we + # fall back to the GIT_COMMIT build-arg/env var (see Dockerfile.backend/playbook.yml). cmd = ['git', '--git-dir=/git'] if os.path.isdir('/git') else ['git'] try: return subprocess.check_output( @@ -36,9 +21,6 @@ def _git_commit(): dotenv.load_dotenv(BASE_DIR / '.env') -# Quick-start development settings - unsuitable for production -# See https://docs.djangoproject.com/en/4.1/howto/deployment/checklist/ - SECRET_KEY = os.environ.get('SECRET_KEY', None) if SECRET_KEY is None: raise Exception('environment variable SECRET_KEY not set. try running `configure.py` or setting it manually') @@ -127,9 +109,6 @@ TEMPLATES = [ WSGI_APPLICATION = 'backend.wsgi.application' -# Database -# https://docs.djangoproject.com/en/4.1/ref/settings/#databases - DATABASES = { 'default': { 'ENGINE': 'django.db.backends.sqlite3', @@ -139,9 +118,6 @@ DATABASES = { AUTH_USER_MODEL = 'authentication.ToolshedUser' -# Password validation -# https://docs.djangoproject.com/en/4.1/ref/settings/#auth-password-validators - AUTH_PASSWORD_VALIDATORS = [ { 'NAME': 'django.contrib.auth.password_validation.UserAttributeSimilarityValidator', @@ -157,9 +133,6 @@ AUTH_PASSWORD_VALIDATORS = [ }, ] -# Internationalization -# https://docs.djangoproject.com/en/4.1/topics/i18n/ - LANGUAGE_CODE = 'en-us' TIME_ZONE = 'UTC' @@ -168,26 +141,18 @@ USE_I18N = True USE_TZ = True -# Static files (CSS, JavaScript, Images) -# https://docs.djangoproject.com/en/4.1/howto/static-files/ - STATIC_ROOT = 'staticfiles' STATIC_URL = '/static/' MEDIA_ROOT = os.environ.get('TOOLSHED_USERFILES_PATH', 'userfiles') MEDIA_URL = '/media/' -# In prod, nginx (running as www-data) reads these files directly - see +# Pinned explicitly (rather than left to the backend process's ambient umask) so group-read +# is guaranteed for nginx/www-data regardless of how the container is started - see # SERVE_X_ACCEL_REDIRECT and playbook.yml's `location /redirect_media/`. -# Pinned explicitly rather than left to the backend process's ambient umask, -# so group-read (www-data is added to the backend's system group) is -# guaranteed regardless of how the container is started. FILE_UPLOAD_PERMISSIONS = 0o640 FILE_UPLOAD_DIRECTORY_PERMISSIONS = 0o750 -# Default primary key field type -# https://docs.djangoproject.com/en/4.1/ref/settings/#default-auto-field - DEFAULT_AUTO_FIELD = 'django.db.models.BigAutoField' STORAGES = { diff --git a/backend/backend/urls.py b/backend/backend/urls.py index 495aac1..105a175 100644 --- a/backend/backend/urls.py +++ b/backend/backend/urls.py @@ -1,18 +1,3 @@ -"""backend URL Configuration - -The `urlpatterns` list routes URLs to views. For more information please see: - https://docs.djangoproject.com/en/4.1/topics/http/urls/ -Examples: -Function views - 1. Add an import: from my_app import views - 2. Add a URL to urlpatterns: path('', views.home, name='home') -Class-based views - 1. Add an import: from other_app.views import Home - 2. Add a URL to urlpatterns: path('', Home.as_view(), name='home') -Including another URLconf - 1. Import the include() function: from django.urls import include, path - 2. Add a URL to urlpatterns: path('blog/', include('blog.urls')) -""" from django.contrib import admin from django.urls import path, include from drf_yasg import openapi diff --git a/backend/backend/wsgi.py b/backend/backend/wsgi.py index c4aa324..21d383b 100644 --- a/backend/backend/wsgi.py +++ b/backend/backend/wsgi.py @@ -1,12 +1,3 @@ -""" -WSGI config for backend project. - -It exposes the WSGI callable as a module-level variable named ``application``. - -For more information on this file, see -https://docs.djangoproject.com/en/4.1/howto/deployment/wsgi/ -""" - import os from django.core.wsgi import get_wsgi_application diff --git a/backend/configure.py b/backend/configure.py index cb04c8c..dde345c 100755 --- a/backend/configure.py +++ b/backend/configure.py @@ -32,11 +32,8 @@ def yesno(prompt, default=False): def configure(): - # Keys this function may generate/update, tracked so that if .env turns - # out to be unwritable (e.g. a prod container running as an unprivileged - # user, whose real config comes from --env-file instead) we can still - # print the resulting configuration for the operator to apply manually, - # rather than silently discarding it or crashing. + # Keys this function may generate/update; tracked so an unwritable .env (e.g. a prod + # container configured via --env-file) can still print them for the operator to apply manually. tracked_keys = ['SECRET_KEY', 'ALLOWED_HOSTS'] unwritable = False diff --git a/backend/files/media_urls.py b/backend/files/media_urls.py index 7c5e2ff..fe8bdc7 100644 --- a/backend/files/media_urls.py +++ b/backend/files/media_urls.py @@ -24,9 +24,8 @@ THUMBNAIL_SIZES = (32, 64, 256) def _accessible_files(request): - # Shared by media_urls and thumbnail_urls so both endpoints always agree on who can see - # what - a file is visible if the requester is friends-or-self with whatever currently - # references it (an inventory item, a profile picture) or it's their own staged photo. + # Shared by media_urls and thumbnail_urls: a file is visible if the requester is + # friends-or-self with whatever references it (item, profile picture), or it's their own staged photo. return File.objects.filter( Q(connected_items__owner__in=request.user.friends_or_self()) | Q(profile_picture_users__in=request.user.friends_or_self()) | @@ -35,8 +34,7 @@ def _accessible_files(request): def _cache_headers(etag): - # Content is addressed by its own hash and can never change under a given URL, so caches - # (and the conditional-GET checks in both views below) can treat it as immutable forever. + # Content is hash-addressed and can never change under a given URL, so it's cacheable forever. return { 'ETag': etag, 'Cache-Control': 'max-age=31536000, private, immutable', @@ -49,23 +47,16 @@ def _cache_headers(etag): @permission_classes([IsAuthenticated]) @authentication_classes([SignatureAuthentication]) def media_urls(request, hash_path): - # Note: CORS headers are NOT set here - django-cors-headers (CorsMiddleware, - # configured in settings.py) adds them to every Django response automatically, so - # setting them manually on these responses would just be redundant. The one exception - # is the SERVE_X_ACCEL_REDIRECT path: nginx replaces this response entirely when it - # follows the X-Accel-Redirect and serves the file itself, so the CORS header for that - # case has to be configured in nginx's `location /redirect_media/` block instead. + # CORS is added automatically by middleware, except via X-Accel-Redirect, where nginx's + # /redirect_media/ block must set it instead. # - # Looked up by the derived storage path (not by raw hash) because FileSerializer.name - # (files/serializers.py) - used everywhere a file URL is handed to the frontend, e.g. - # AuthenticatedImage's `src` - already returns this path via Django's FileField.url, and - # the existing test suite (files/tests.py MediaUrlTestCase) exercises it this way too. + # Looked up by the derived storage path, not the raw hash, to match FileSerializer.name + # (used for AuthenticatedImage's `src`) and the existing test suite (MediaUrlTestCase). try: file = _accessible_files(request).get(file=hash_path) - # The access-control lookup above must happen before this check - otherwise a bare - # hash + If-None-Match would let anyone probe "does a file with this hash exist" for - # files they can't actually see. + # Must run before this check, else a bare hash + If-None-Match would let anyone probe + # file existence for files they can't see. if request.META.get('HTTP_IF_NONE_MATCH') == file.hash: return HttpResponse(status=status.HTTP_304_NOT_MODIFIED) @@ -79,9 +70,7 @@ def media_urls(request, hash_path): **cache_headers, }) else: - # Read via the FieldFile itself (works against whatever storage backend is - # actually configured) rather than assuming file.file.path is a real filesystem - # path - the test suite swaps in an in-memory backend where that isn't true. + # Reads via FieldFile.open() (not file.file.path) since tests swap in an in-memory storage backend. with file.file.open('rb') as fh: content = fh.read() return HttpResponse(status=status.HTTP_200_OK, @@ -94,9 +83,7 @@ def media_urls(request, hash_path): def _thumbnail_rel_path(file_hash, size): - # Mirrors files/models.py's hash_upload() sharding, under its own `thumbnails//` - # subtree - reachable through the same nginx `/redirect_media/` alias as originals when - # served from real disk, no separate nginx location needed. + # Mirrors hash_upload()'s sharding under thumbnails//, reachable via the same nginx alias as originals. return os.path.join('thumbnails', str(size), file_hash[:2], file_hash[2:4], file_hash[4:6], file_hash[6:] + '.jpg') @@ -116,22 +103,15 @@ def thumbnail_urls(request, size, hash_path): if request.META.get('HTTP_IF_NONE_MATCH') == etag: return HttpResponse(status=status.HTTP_304_NOT_MODIFIED) - # Read/write through the default storage backend, same as File.file itself, rather - # than a hand-rolled filesystem path - correct regardless of storage backend (real - # disk in production, in-memory under the test runner) and keeps the cache in the - # same place originals live. + # Read/write via default_storage, not a hand-rolled path, to work with both real-disk + # and in-memory test storage. rel_path = _thumbnail_rel_path(file.hash, size) if not default_storage.exists(rel_path): - # Thumbnails are always re-encoded as JPEG regardless of the original format - - # smaller and simpler than preserving e.g. PNG transparency at this scale. + # Always re-encoded as JPEG regardless of original format - simpler than preserving transparency at this scale. with file.file.open('rb') as fh: image = Image.open(fh) image.thumbnail((size, size)) - # Flatten through RGBA before dropping to RGB - some modes (grayscale+alpha, - # palette-with-transparency, RGBA) store meaningless color/luminance data under - # fully transparent pixels (often zeroed out, i.e. black). Converting straight - # to RGB reveals that instead of "nothing there"; compositing onto an opaque - # background first shows what the image is actually supposed to look like. + # Flatten through RGBA before dropping to RGB. See docs/implementation.md#rgba-flattening-avoids-revealing-black-under-transparent-pixels. rgba = image.convert('RGBA') flattened = Image.new('RGB', rgba.size, (255, 255, 255)) flattened.paste(rgba, mask=rgba.getchannel('A')) diff --git a/backend/files/models.py b/backend/files/models.py index bff9121..a55f794 100644 --- a/backend/files/models.py +++ b/backend/files/models.py @@ -43,15 +43,7 @@ class FileManager(models.Manager): else: raise ValueError('data must be a base64 encoded string or file and hash must be provided') if not self.filter(hash=kwargs['hash']).exists(): - # The upload path is derived entirely from the hash (hash_upload, above), and hash - # is DB-unique - so if no File row owns this hash yet, anything already sitting at - # its computed path is necessarily a stale orphan (e.g. left behind by a bug in - # cleanup code that deleted a File row without removing its stored bytes, or a - # crashed upload). Clear it before saving instead of letting Django's storage layer - # invent an alternate filename to avoid the "collision" - a suffixed name would - # silently break every part of the app that derives this file's URL purely from its - # hash (media serving, thumbnail generation, item/avatar attachment), and no future - # caller could ever discover it again. + # Clears a stale orphan already at this hash's canonical path before saving. See docs/implementation.md#stale-orphan-cleanup-at-the-canonical-hash-path. expected_path = hash_upload(SimpleNamespace(hash=kwargs['hash']), '') if default_storage.exists(expected_path): default_storage.delete(expected_path) diff --git a/backend/files/tests.py b/backend/files/tests.py index 05ecb38..8c160e9 100644 --- a/backend/files/tests.py +++ b/backend/files/tests.py @@ -113,12 +113,7 @@ class FilesTestCase(FilesTestMixin, ToolshedTestCase): self.assertEqual(countdir(DefaultStorage(), ''), 3) def test_file_upload_reclaims_stale_orphan_at_canonical_path(self): - # Reproduces a real incident: a File row gets deleted without its underlying stored - # bytes being removed (e.g. a bug in some cleanup call site), leaving an orphan sitting - # at the exact path hash_upload() would compute for that content. A later upload of the - # same content must land back on that canonical path - not get silently suffixed by - # Django's default collision-avoidance, which would make it unreachable to every part - # of the app that derives a file's URL purely from its hash. + # Regression test for a stale orphan at the canonical hash path. See docs/implementation.md#stale-orphan-cleanup-at-the-canonical-hash-path. expected_path = f"{self.f['hash4'][:2]}/{self.f['hash4'][2:4]}/{self.f['hash4'][4:6]}/{self.f['hash4'][6:]}" default_storage.save(expected_path, ContentFile(self.f['test_content4'])) self.assertTrue(default_storage.exists(expected_path)) @@ -231,11 +226,8 @@ class ThumbnailUrlTestCase(FilesTestMixin, UserTestMixin, InventoryTestMixin, To self.prepare_properties() self.prepare_inventory() - # Each test method gets its own distinct image content (and therefore its own content - # hash / thumbnail cache path) - InMemoryStorage isn't reset between test methods within - # a run, so sharing one fixed image across methods risks one test's cached (or, in - # test_thumbnail_served_from_cache_on_second_request's case, deliberately corrupted) - # thumbnail leaking into another test's assertions. + # Each test uses a distinct seeded image (own hash/cache path) since InMemoryStorage + # isn't reset between test methods, so a shared image risks one test's cached thumbnail leaking into another's assertions. seed = zlib.crc32(self._testMethodName.encode()) % 256 buffer = io.BytesIO() Image.new('RGB', (800, 600), (seed, 255 - seed, 128)).save(buffer, 'PNG') @@ -254,8 +246,7 @@ class ThumbnailUrlTestCase(FilesTestMixin, UserTestMixin, InventoryTestMixin, To return os.path.join('thumbnails', str(size), h[:2], h[2:4], h[4:6], h[6:] + '.jpg') def test_thumbnail_sizes_available(self): - # Documents the fixed size allow-list this test suite exercises against - update both - # if files/media_urls.py's THUMBNAIL_SIZES ever changes. + # Fixed size allow-list this suite exercises - update both if media_urls.py's THUMBNAIL_SIZES changes. self.assertEqual(THUMBNAIL_SIZES, (32, 64, 256)) @override_settings(SERVE_X_ACCEL_REDIRECT=False) @@ -274,10 +265,7 @@ class ThumbnailUrlTestCase(FilesTestMixin, UserTestMixin, InventoryTestMixin, To @override_settings(SERVE_X_ACCEL_REDIRECT=False) def test_thumbnail_flattens_transparency_instead_of_going_black(self): - # Reproduces a real incident: an 'LA' (grayscale + alpha) source whose fully-transparent - # region has zeroed-out luminance underneath, as many image tools produce. Converting - # straight to RGB (dropping alpha without compositing) reveals that zeroed data - the - # whole thumbnail comes out solid black even though the visible (opaque) content isn't. + # Regression test for an 'LA' source with zeroed transparent-region luminance. See docs/implementation.md#rgba-flattening-avoids-revealing-black-under-transparent-pixels. half_transparent = Image.new('LA', (200, 200)) pixels = half_transparent.load() for x in range(200): @@ -310,8 +298,7 @@ class ThumbnailUrlTestCase(FilesTestMixin, UserTestMixin, InventoryTestMixin, To with default_storage.open(rel_path, 'rb') as f: cached_bytes = f.read() - # Overwrite the cached file with a marker so a correct implementation must serve this - # exact content back rather than regenerating it from the original. + # Overwrites the cache with a marker so a correct implementation must serve it back, not regenerate. default_storage.delete(rel_path) default_storage.save(rel_path, ContentFile(cached_bytes + b'MARKER')) @@ -333,8 +320,7 @@ class ThumbnailUrlTestCase(FilesTestMixin, UserTestMixin, InventoryTestMixin, To self.assertEqual(reply.status_code, 403) def test_thumbnail_not_friend(self): - # local_user1/local_user2 are friends in these fixtures (see prepare_inventory) - the - # denied case needs a stranger to that friendship instead. + # local_user1/local_user2 are friends here (see prepare_inventory), so the denied case needs a stranger instead. reply = client.get(self._thumb_url(64), self.f['ext_user1']) self.assertEqual(reply.status_code, 404) self.assertFalse(default_storage.exists(self._thumb_rel_path(64))) diff --git a/backend/toolshed/api/files.py b/backend/toolshed/api/files.py index 9b3b8d0..03b51d9 100644 --- a/backend/toolshed/api/files.py +++ b/backend/toolshed/api/files.py @@ -50,10 +50,7 @@ def post_item_file(request, item_id): if item is None: return Response(status=status.HTTP_404_NOT_FOUND) if 'file_hash' in request.data: - # Attach a file the caller already staged on one of their own workflows, identified - # by its content hash (which the client already computed before ever uploading it), - # instead of re-uploading bytes that are already stored server-side. Workflows are - # always personally owned, so this only applies to a caller with a local account. + # Attaches an already-staged file by hash instead of re-uploading it. See docs/implementation.md#staged-files-are-identified-by-hash-alone. if not request.user.user.exists(): return Response(status=status.HTTP_404_NOT_FOUND) try: @@ -74,10 +71,7 @@ def post_item_file(request, item_id): def get_staged_files(request, workflow_id): try: workflow = WorkflowInstance.objects.get(id=workflow_id, owner=request.user) - # Hash alone identifies a staged file (client and server hash content the same way, and - # bytes are fetchable from a hash-derived storage path) - useful mainly for discovering - # what another session/device already staged on this workflow, unlike the fuller - # FileSerializer representation item_files uses. + # Hash alone is enough to discover what another session/device already staged. See docs/implementation.md#staged-files-are-identified-by-hash-alone. return Response(list(workflow.staged_files.values_list('hash', flat=True))) except WorkflowInstance.DoesNotExist: return Response(status=status.HTTP_404_NOT_FOUND) diff --git a/backend/toolshed/api/inventory.py b/backend/toolshed/api/inventory.py index c0c119c..40f2b97 100644 --- a/backend/toolshed/api/inventory.py +++ b/backend/toolshed/api/inventory.py @@ -35,26 +35,22 @@ class InventoryItemViewSet(viewsets.ModelViewSet): serializer_class = InventoryItemSerializer authentication_classes = [SignatureAuthentication] permission_classes = [IsAuthenticated] - # Detail routes address an item by its owner-scoped id, not the internal row id - the - # router still names the URL capture group 'pk', so keep that as lookup_url_kwarg and just - # change which model field it's matched against. get_queryset() below is always already - # scoped to the requester's own items/groups, so this can't cross into another owner's ids. + # Detail routes address an item 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): - # A KnownIdentity acting purely as a group member (e.g. a remote member on a group - # hosted on this backend) never has a local ToolshedUser account here - group-owned - # items must stay reachable for such an identity, only personal ("owner=...") items - # require .user.exists(). + # A pure group-member KnownIdentity may have no local ToolshedUser account; only + # personal items require .user.exists(). See + # docs/implementation.md#group-member-identities-without-local-accounts. if type(self.request.user) != KnownIdentity: return InventoryItem.objects.none() identity = self.request.user group_items = InventoryItem.objects.filter(owner_group__in=identity.member_of_groups.all()) if self.action != 'list': - # retrieve/update/destroy: anything the caller may act on - their own items, or any - # group they're currently a member of. The narrower per-group listing below is only - # for the list action, so the main Inventory page stays scoped to personal items. + # retrieve/update/destroy: any item the caller may act on, own or group. See + # docs/implementation.md#inventory-queryset-scope-by-action. if identity.user.exists(): return InventoryItem.objects.filter(owner=identity.user.get()) | group_items return group_items @@ -127,13 +123,8 @@ def search_inventory_items(request): @authentication_classes([SignatureAuthentication]) @permission_classes([IsAuthenticated]) def get_shared_item(request, handle, id): - """Fetch a single item by its owner's handle (username@domain) and local id, e.g. for the - /i// item URL (see docs/design-in-progress/items-labels.md) or the - /inventory/shared// in-app view. Unlike InventoryItemViewSet, which only ever - returns the requester's own items, this looks the item up by owner instead of by requester, - so it's the only endpoint that can serve a friend's item - subject to the same - friends-or-self and availability_policy checks getUserProfile/_accessible_files already - use elsewhere.""" + """Fetch a single item by its owner's handle and local id, for /i// or + /inventory/shared//. See docs/implementation.md#get-shared-item-looks-up-by-owner.""" try: username, domain = split_userhandle_or_throw(handle) except ValueError: diff --git a/backend/toolshed/models.py b/backend/toolshed/models.py index d02de2b..6fdb9ff 100644 --- a/backend/toolshed/models.py +++ b/backend/toolshed/models.py @@ -83,9 +83,9 @@ class Tag(models.Model): class OwnerItemSequence(models.Model): - """Tracks the last InventoryItem id handed out to a given owner or owner_group, so ids can - be allocated sequentially and without gaps within that scope (see InventoryItem.create_for_owner). - Exactly one of owner/owner_group is set, mirroring InventoryItem's own owner/owner_group split.""" + """Tracks the last InventoryItem id handed out per owner/owner_group scope for sequential, + gapless allocation (see InventoryItem.create_for_owner); exactly one of owner/owner_group is + set, mirroring InventoryItem's own split.""" owner = models.ForeignKey(ToolshedUser, on_delete=models.CASCADE, null=True, blank=True, related_name='+') owner_group = models.ForeignKey(Group, on_delete=models.CASCADE, null=True, blank=True, related_name='+') last_id = models.PositiveIntegerField(default=0) @@ -117,9 +117,8 @@ class InventoryItem(SoftDeleteModel): ) internal_id = models.AutoField(primary_key=True) - # The externally visible identifier: sequential and gapless within owner/owner_group's own - # items (see OwnerItemSequence), never the internal_id above. Always allocate through - # create_for_owner rather than InventoryItem.objects.create() directly. + # Externally visible id, sequential/gapless within owner/owner_group's own items (see + # OwnerItemSequence), never internal_id; always allocate via create_for_owner, not .objects.create(). id = models.PositiveIntegerField(editable=False) published = models.BooleanField(default=False) name = models.CharField(max_length=255, null=True, blank=True) @@ -152,8 +151,8 @@ class InventoryItem(SoftDeleteModel): @classmethod def create_for_owner(cls, *, owner=None, owner_group=None, **kwargs): - """The only supported way to create an InventoryItem: allocates the next id for this - owner/owner_group scope and creates the item with it, atomically.""" + """The only supported way to create an InventoryItem: atomically allocates the next id + for this owner/owner_group scope.""" with transaction.atomic(): next_id = OwnerItemSequence.allocate(owner=owner, owner_group=owner_group) return cls.objects.create(owner=owner, owner_group=owner_group, id=next_id, **kwargs) diff --git a/backend/toolshed/offlinedata.py b/backend/toolshed/offlinedata.py index 10cde6e..1175e63 100644 --- a/backend/toolshed/offlinedata.py +++ b/backend/toolshed/offlinedata.py @@ -1,9 +1,4 @@ -"""Data helpers for building/importing a user's offline export. - -These functions deal with the toolshed models and CSV row shapes only - they know nothing -about the zip container format or how the request is authenticated. See toolshed/api/offlinedata.py -for the zip-building/parsing and the API views themselves. -""" +"""Data helpers for building/importing a user's offline export; deal with toolshed models and CSV row shapes only, and know nothing about the zip container format or request auth - see toolshed/api/offlinedata.py for that.""" def inventory_rows(user): @@ -76,11 +71,7 @@ def location_rows(user): def inventory_files(user): - """Generator that yields (arcname, data) for each unique file attached to the user's inventory items. - - Files are deduplicated by hash and placed under a 'files/' subfolder, keeping their original - extension (guessed from mime_type) so attachments and images remain viewable once extracted. - """ + """Yields (arcname, data) for each unique file attached to the user's inventory items, deduplicated by hash. See docs/implementation.md#file-naming-convention-in-exports for the naming scheme.""" import mimetypes from toolshed.models import InventoryItem @@ -125,11 +116,7 @@ def profile_data(user): def profile_picture_files(user): - """Generator that yields (arcname, data) for the user's profile picture, if one is set. - - Kept separate from `inventory_files()` so the profile picture is included in the export - even for users with no inventory items or whose picture isn't attached to any item. - """ + """Generator that yields (arcname, data) for the user's profile picture, if one is set. Kept separate from `inventory_files()` so the picture is included even for users with no inventory items or whose picture isn't attached to any item.""" import mimetypes if not user.profile_picture: @@ -148,21 +135,12 @@ def profile_picture_files(user): def settings_data(user): - """Return the given user's account-level preferences as a {key: value} dict for settings.json. - - Only account preferences (AccountPreference) are exported; device-level preferences stay - in the browser's local storage since they describe the device, not the account. - """ + """Return the given user's account-level preferences as a {key: value} dict for settings.json. Only account preferences are exported; device-level preferences stay in the browser's local storage since they describe the device, not the account.""" return {pref.key: pref.value for pref in user.preferences.all()} def import_profile(user, data, available_files): - """Fault-tolerant import of profile.json, updating the user's editable profile fields. - - Only 'first_name', 'last_name', 'email' and 'profile_picture' (a 'files/...' path present in - `available_files`) are applied; 'username' and 'domain' are ignored since they identify the - account itself and can't be changed by an import. - """ + """Fault-tolerant import of profile.json, updating the user's editable profile fields. See docs/implementation.md#profile-import-semantics for which fields are applied and why.""" import json try: @@ -189,11 +167,7 @@ def import_profile(user, data, available_files): def import_settings(user, data): - """Fault-tolerant import of settings.json into the user's account-level preferences. - - Each top-level key/value pair is upserted as an AccountPreference; unreadable data or a - non-object payload is skipped rather than aborting the whole import. - """ + """Fault-tolerant import of settings.json; each top-level key/value pair is upserted as an AccountPreference, and unreadable data or a non-object payload is skipped rather than aborting the whole import.""" import json from authentication.models import AccountPreference @@ -217,18 +191,7 @@ def import_settings(user, data): def delete_user_data(user): - """Permanently delete everything that `user_data()` exports, keeping the account itself intact. - - This removes the user's inventory items (hard delete, bypassing soft-delete), storage - locations, account preferences, the friends relation on their public identity, the profile - picture, and any File blobs (profile picture / inventory attachments) that would otherwise be - orphaned - i.e. not referenced by any other inventory item or user. Files still referenced - elsewhere (they're deduplicated by content hash) are left untouched. The ToolshedUser account - (and its underlying KnownIdentity) is *not* deleted - this only wipes the account's data, it - doesn't close the account. See `delete_account()` in toolshed/api/offlinedata.py for that. - - Returns a summary dict describing what was removed. - """ + """Permanently delete everything that `user_data()` exports, keeping the account itself intact. Returns a summary dict describing what was removed. See docs/implementation.md#account-data-deletion for exactly what's removed and why the account itself survives.""" from django.db import transaction from toolshed.models import InventoryItem, StorageLocation @@ -262,15 +225,7 @@ def delete_user_data(user): def delete_user_account(user): - """Permanently delete the local user's account, after wiping all of its data. - - This first calls `delete_user_data()` to remove everything covered by the data export - (inventory, locations, settings, friends relation, profile picture and orphaned files), - then deletes the ToolshedUser row itself. The underlying KnownIdentity is kept so remote - friends/history relating to this identity remain intact - only the local account is closed. - - Returns a summary dict describing what was removed, with `account` set to True. - """ + """Permanently delete the local user's account, after wiping all of its data via `delete_user_data()`. Returns a summary dict with `account` set to True. See docs/implementation.md#account-data-deletion for why the underlying KnownIdentity is kept.""" from django.db import transaction with transaction.atomic(): @@ -282,11 +237,7 @@ def delete_user_account(user): def _delete_orphaned_files(file_ids): - """Delete File rows (and their underlying blobs) in `file_ids` that are no longer referenced. - - A File is considered orphaned once no InventoryItem, no ToolshedUser (profile picture), and no - WorkflowInstance (staged file) references it anymore. Returns the number of files deleted. - """ + """Delete File rows (and their underlying blobs) in `file_ids` that are no longer referenced. Returns the number of files deleted. See docs/implementation.md#account-data-deletion for the orphan definition.""" from files.models import File deleted = 0 @@ -360,15 +311,7 @@ def get_or_create_category(path): def import_locations(user, data): - """Fault-tolerant import of locations.csv into StorageLocations owned by `user`. - - Rows are read by header label. Rows missing the required 'name' column are skipped. - Optional columns ('description', 'category') are simply omitted if absent. Locations are - processed in path-depth order so a child's parent already exists by the time it's needed. - Each row runs in its own transaction savepoint so a DB-level failure on one row (e.g. a - constraint violation) can't poison the surrounding transaction and silently break every - subsequent row. - """ + """Fault-tolerant import of locations.csv into StorageLocations owned by `user`. See docs/implementation.md#location-import-ordering-and-savepoints for row ordering and error-isolation rules.""" from django.db import transaction from toolshed.models import StorageLocation @@ -444,22 +387,11 @@ def import_friends(user, data): class _HandleNotFound(Exception): - """Raised when a fully qualified handle (e.g. 'git:base#tag:drill') can't be resolved. - - This intentionally aborts the whole row (rather than falling back to creating a new - entity), since a handle references a *specific* entity from a *specific* origin - silently - creating a new local entity named after the raw handle string would be incorrect. - """ + """Raised when a fully qualified handle (e.g. 'git:base#tag:drill') can't be resolved, aborting the row rather than creating a new entity. See docs/implementation.md#handle-resolution-semantics.""" def _resolve_handle(value, entity_type, model): - """Resolve a fully qualified handle (e.g. 'git:base#tag:drill') to an *existing* model instance. - - Never creates anything: a handle references a specific entity from a specific origin, so - silently creating a new local entity named after the raw handle string would be incorrect. - Raises `_HandleNotFound` if the handle's entity type doesn't match or no such object exists, - so the caller can skip the row and report a helpful error instead. - """ + """Resolve a fully qualified handle (e.g. 'git:base#tag:drill') to an *existing* model instance; never creates one. See docs/implementation.md#handle-resolution-semantics for the rationale.""" origin, rest = value.split('#', 1) if ':' in rest: found_type, name = rest.split(':', 1) @@ -475,20 +407,14 @@ def _resolve_handle(value, entity_type, model): def _quote_value_if_needed(value): - """Wrap `value` in double quotes (CSV-style, doubling any embedded quotes) if it contains a - comma or a quote character, so it survives sitting inside a comma-separated "handle=value" - list unambiguously. - """ + """Wrap `value` in double quotes (CSV-style, doubling embedded quotes) if it contains a comma or quote character. See docs/implementation.md#properties-csv-encoding.""" if any(ch in value for ch in ',"'): return '"' + value.replace('"', '""') + '"' return value def _encode_properties_cell(item_properties): - """Encode an item's properties as a comma-separated "handle=value" list for the 'properties' - CSV cell, quoting a value (CSV-style) when it contains a comma or a quote character so it - still round-trips correctly. See `_parse_properties_cell()` for the reader side. - """ + """Encode an item's properties as a comma-separated "handle=value" list for the 'properties' CSV cell. See docs/implementation.md#properties-csv-encoding and `_parse_properties_cell()` for the reader side.""" entries = [ f"{ip.property.get_handle()}={_quote_value_if_needed(ip.value or '')}" for ip in item_properties @@ -497,15 +423,7 @@ def _encode_properties_cell(item_properties): def _split_quoted_comma_list(raw_value): - """Split a comma-separated list into entries, honouring double-quoted substrings (CSV-style, - wherever they appear in an entry) so a quoted value's own commas aren't mistaken for - separators. Doubled quotes ("") inside a quoted substring are unescaped to a single literal - quote, and the surrounding quotes themselves are stripped from the result. - - Only the single space after each ", " separator (as written by `_encode_properties_cell()`) - is dropped - unlike a blanket `.strip()`, this preserves any leading/trailing whitespace that - is genuinely part of a (quoted) value. - """ + """Split a comma-separated list into entries, honouring double-quoted substrings (CSV-style) so a quoted value's own commas aren't mistaken for separators. See docs/implementation.md#properties-csv-encoding for the quoting/whitespace rules this implements.""" entries = [] current = [] in_quotes = False @@ -537,12 +455,7 @@ def _split_quoted_comma_list(raw_value): def _parse_properties_cell(raw_value, resolve_property): - """Parse the 'properties' CSV cell into a list of (Property, value) tuples. - - The cell is a comma-separated list of "handle=value" entries; a value containing a comma or - a quote character is double-quoted (CSV-style) by `_encode_properties_cell()` so it survives - the round trip intact. - """ + """Parse the 'properties' CSV cell into a list of (Property, value) tuples. See docs/implementation.md#properties-csv-encoding for how the cell is encoded by `_encode_properties_cell()`.""" raw_value = (raw_value or '').strip() if not raw_value: return [] @@ -561,17 +474,7 @@ def _parse_properties_cell(raw_value, resolve_property): def import_inventory(user, data, available_files): - """Fault-tolerant import of inventory.csv into InventoryItems owned by `user`. - - `available_files` maps the 'files' column's paths to File instances successfully extracted - from the zip; references to missing files are ignored rather than failing the row. - - If a row references a fully qualified tag/property/category handle that doesn't exist - locally, the whole item is skipped (rather than creating a bogus local entity named after - the raw handle) and a helpful message is added to the returned `errors` list. Each row runs - in its own transaction savepoint so a DB-level failure on one row can't poison the - surrounding transaction and silently break every subsequent row. - """ + """Fault-tolerant import of inventory.csv into InventoryItems owned by `user`. See docs/implementation.md#inventory-import-semantics for file/handle resolution and error-isolation rules.""" from django.db import transaction from toolshed.models import Category, InventoryItem, ItemProperty, StorageLocation, Tag, Property diff --git a/backend/toolshed/serializers.py b/backend/toolshed/serializers.py index 3cf9a03..4c864f3 100644 --- a/backend/toolshed/serializers.py +++ b/backend/toolshed/serializers.py @@ -255,11 +255,7 @@ class InventoryItemSerializer(serializers.ModelSerializer): class WorkflowInstanceSerializer(serializers.ModelSerializer): owner = serializers.StringRelatedField(read_only=True) - # Hash is enough to identify a staged file (the client computes the same SHA-256 the backend - # does, and can fetch bytes from a hash-derived storage path) - for anything staged by *this* - # session there's nothing more to say, and for a file staged elsewhere (another device/tab), - # hash is what lets this session recognize and fetch it. Unlike InventoryItemSerializer.files, - # no fuller FileSerializer representation is needed here. + # Only the hash is needed to identify a staged file, unlike InventoryItemSerializer.files. See docs/implementation.md#staged-files-are-identified-by-hash-alone. staged_files = serializers.SerializerMethodField() class Meta: diff --git a/backend/toolshed/tests/test_friend.py b/backend/toolshed/tests/test_friend.py index fba1a42..202030b 100644 --- a/backend/toolshed/tests/test_friend.py +++ b/backend/toolshed/tests/test_friend.py @@ -85,20 +85,8 @@ class FriendApiTestCase(UserTestMixin, ToolshedTestCase): self.assertEqual(self.f['local_user1'].friends.count(), 1) -# what ~should~ happen: -# 1. user x@A sends a friend request to user y@B -# 1.1. x@A's client sends a POST request to A/api/friendrequests/ with body {from: x@A, to: y@B} -# 1.2. A's backend creates a FriendRequestOutgoing object, containing x@A's identity and y@B's name -# 1.3. x@A's client sends a POST request to B/api/friendrequests/ with body -# {from: x@A, to: y@B, public_key: x@A's public key} -# 1.4. B's backend creates a FriendRequestIncoming object, containing y@B's and x@A's identities -# 2. user y@B accepts the friend request -# 2.1. y@B's client sends a POST request to A/api/friendsrequests/ with body -# {from: x@A, to: y@B, public_key: y@B's public key} -# 2.2. A's backend matches the data to the FriendRequestOutgoing object, deletes both and creates a Friend object, -# containing x@A's and y@B's identities -# 2.3. y@B's client sends a POST request to B/api/friends/ containing the id of the FriendRequestIncoming object -# 2.4. B's backend creates a Friend object, using the identities from the FriendRequestIncoming object +# Friend request/accept protocol walkthrough. See +# docs/implementation.md#friend-request-and-accept-protocol-flow. class FriendRequestListTestCase(UserTestMixin, ToolshedTestCase): diff --git a/backend/toolshed/tests/test_inventory.py b/backend/toolshed/tests/test_inventory.py index dc608ba..eba0c12 100644 --- a/backend/toolshed/tests/test_inventory.py +++ b/backend/toolshed/tests/test_inventory.py @@ -388,9 +388,8 @@ class GroupOwnedInventoryApiTestCase(UserTestMixin, GroupTestMixin, CategoryTest self.assertEqual(InventoryItem.objects.filter(id=item_id).count(), 0) def test_remote_member_without_local_account_can_edit(self): - # A remote member (no ToolshedUser row at all on this backend, only a KnownIdentity - - # see docs/design-in-progress/groups-mvp.md) must still be able to act on group-owned - # items here; it must not be treated as unauthorized just because .user.exists() is False. + # A remote member (KnownIdentity, no ToolshedUser row) must still act on group-owned + # items - not unauthorized just because .user.exists() is False. self.f['group1'].members.add(self.f['ext_user1'].public_identity) item_id = self.create_group_item().json()['id'] reply = client.get('/api/inventory_items/{}/'.format(item_id), self.f['ext_user1']) @@ -426,8 +425,8 @@ class GroupOwnedInventoryApiTestCase(UserTestMixin, GroupTestMixin, CategoryTest self.assertEqual(len(reply.json()), 0) def test_create_group_owned_item_with_full_fields(self): - # Parity with InventoryApiTestCase.test_post_new_item - tags/properties/category must - # attach to a group-owned item exactly the same way they do for a personal one. + # Parity with InventoryApiTestCase.test_post_new_item: tags/properties/category attach + # to a group-owned item the same way as a personal one. reply = client.post('/api/inventory_items/', self.f['local_user1'], { 'availability_policy': 'rent', 'category': 'cat2', @@ -450,8 +449,7 @@ class GroupOwnedInventoryApiTestCase(UserTestMixin, GroupTestMixin, CategoryTest self.assertEqual([p.value for p in item.itemproperty_set.all()], ['value1', 'value2']) def test_create_group_owned_item_empty_fails(self): - # Parity with InventoryApiTestCase.test_post_new_item_empty - clean()'s name-or-files - # validation must still apply to group-owned items. + # Parity with InventoryApiTestCase.test_post_new_item_empty: clean()'s name-or-files validation still applies. reply = client.post('/api/inventory_items/', self.f['local_user1'], { 'availability_policy': 'private', 'owned_quantity': 1, 'owner_group': self.f['group1'].id, }) @@ -471,9 +469,8 @@ class GroupOwnedInventoryApiTestCase(UserTestMixin, GroupTestMixin, CategoryTest self.assertEqual(len(reply.json()), 0) def test_put_group_item(self): - # Parity with InventoryApiTestCase.test_put_item - full replace, by a different member - # than the one who created it, exercising the group_items_id -> _is_authorized branch - # in perform_update for a PUT (not just PATCH). + # Parity with InventoryApiTestCase.test_put_item, but as a PUT by a different member than + # the creator, to exercise the _is_authorized branch in perform_update for PUT too. item_id = self.create_group_item().json()['id'] reply = client.put('/api/inventory_items/{}/'.format(item_id), self.f['local_user2'], { 'availability_policy': 'sell', @@ -539,9 +536,8 @@ class GroupOwnedInventoryApiTestCase(UserTestMixin, GroupTestMixin, CategoryTest self.assertEqual([f for f in item.files.all()], [self.f['test_file3']]) def test_group_items_excluded_from_search(self): - # Group-owned items are only ever reachable via the group's own detail page for MVP - # (see docs/design-in-progress/groups-mvp.md) - search must not surface them, same as - # the main Inventory list already doesn't. + # Group-owned items are reachable only via the group's own detail page for MVP (see + # docs/design-in-progress/groups-mvp.md) - search must not surface them either. self.create_group_item(name='searchable-drill') InventoryItem.create_for_owner(owner=self.f['local_user1'], owned_quantity=1, name='searchable-personal') reply = client.get('/api/search/?query=searchable', self.f['local_user1']) @@ -551,8 +547,8 @@ class GroupOwnedInventoryApiTestCase(UserTestMixin, GroupTestMixin, CategoryTest class InventoryItemIdAllocationTestCase(UserTestMixin, ToolshedTestCase): - """InventoryItem.id is sequential and gapless within each owner/owner_group's own items, - never reused, and allocated independently per scope - see OwnerItemSequence.""" + """InventoryItem.id is sequential, gapless, and never reused within each owner/owner_group's + own items, allocated independently per scope (see OwnerItemSequence).""" def setUp(self): super().setUp() diff --git a/backend/toolshed/tests/test_offlinedata.py b/backend/toolshed/tests/test_offlinedata.py index 655cd38..ac5b55f 100644 --- a/backend/toolshed/tests/test_offlinedata.py +++ b/backend/toolshed/tests/test_offlinedata.py @@ -123,10 +123,7 @@ class DeleteAccountTestCase(_DeleteTestDataMixin, ToolshedTestCase): class ImportInventoryPropertiesTestCase(UserTestMixin, CategoryTestMixin, TagTestMixin, PropertyTestMixin, ToolshedTestCase): - """Properties must round-trip through export/import even when their value contains a - comma or an '=' sign - characters that a naive "handle=value, handle2=value2" encoding of - the 'properties' CSV cell would misinterpret as a field/entry separator. - """ + """Properties must round-trip through export/import even when their value contains a comma or '=' sign, which a naive "handle=value, handle2=value2" encoding would otherwise misinterpret as a separator.""" def setUp(self): super().setUp() @@ -222,10 +219,7 @@ class ImportInventoryPropertiesTestCase(UserTestMixin, CategoryTestMixin, TagTes class ExportImportApiRoundTripTestCase(UserTestMixin, CategoryTestMixin, TagTestMixin, PropertyTestMixin, ToolshedTestCase): - """End-to-end coverage of the /api/export/ + /api/import/ endpoints (as actually used by - clients), rather than calling the internal helper functions directly - this is what a real - export/import round trip between two accounts looks like. - """ + """End-to-end coverage of the /api/export/ + /api/import/ endpoints (as actually used by clients), rather than calling the internal helper functions directly - this is what a real export/import round trip between two accounts looks like.""" def setUp(self): super().setUp() diff --git a/deploy/dev/Dockerfile.proxy b/deploy/dev/Dockerfile.proxy index 095c80f..ececb93 100644 --- a/deploy/dev/Dockerfile.proxy +++ b/deploy/dev/Dockerfile.proxy @@ -1,14 +1 @@ FROM nginx:bookworm - -# snakeoil for localhost - -RUN apt-get update && \ - apt-get install -y openssl && \ - openssl genrsa -des3 -passout pass:x -out server.pass.key 2048 && \ - openssl rsa -passin pass:x -in server.pass.key -out server.key && \ - rm server.pass.key && \ - openssl req -new -key server.key -out server.csr \ - -subj "/C=US/ST=Denial/L=Springfield/O=Dis/CN=localhost" && \ - openssl x509 -req -days 365 -in server.csr -signkey server.key -out server.crt &&\ - mv server.crt /etc/nginx/nginx.crt && \ - mv server.key /etc/nginx/nginx.key \ diff --git a/deploy/dev/docker-compose.yml b/deploy/dev/docker-compose.yml index 30ed12a..473ea58 100644 --- a/deploy/dev/docker-compose.yml +++ b/deploy/dev/docker-compose.yml @@ -72,6 +72,14 @@ services: - ./instance_a/dns.json:/var/www/dns.json:ro - ./instance_a/domains.json:/var/www/domains.json:ro - ./instance_a/userfiles:/var/www/userfiles:ro + # A stable, CA-signed cert (see frontend/.local/make_localhost.sh) covering localhost plus + # every loopback IP a dev proxy binds to below, instead of Dockerfile.proxy generating a + # fresh throwaway self-signed one on every image build - that regenerated cert invalidated + # any trust exception you'd added in your browser on the previous build. Trust + # frontend/.local/RootCA.crt once (see docs/development.md) and it keeps working across + # rebuilds. + - ../../frontend/.local/localhost.crt:/etc/nginx/nginx.crt:ro + - ../../frontend/.local/localhost.key:/etc/nginx/nginx.key:ro ports: - "127.0.0.1:8080:8080" - "127.0.0.3:5353:5353" @@ -83,6 +91,8 @@ services: volumes: - ./instance_b/nginx-b.dev.conf:/etc/nginx/nginx.conf:ro - ./instance_b/userfiles:/var/www/userfiles:ro + - ../../frontend/.local/localhost.crt:/etc/nginx/nginx.crt:ro + - ../../frontend/.local/localhost.key:/etc/nginx/nginx.key:ro ports: - "127.0.0.2:8080:8080" diff --git a/docs/index.md b/docs/index.md index 756dc7e..cb46256 100644 --- a/docs/index.md +++ b/docs/index.md @@ -10,5 +10,6 @@ This is the documentation for the Toolshed project. It is a work in progress. - [Development Setup](development.md) - [About Federation](federation.md) - [Handles and Short IDs](handles-and-shortids.md) + - [Implementation Notes](implementation.md) diff --git a/frontend/src/cameraManager.js b/frontend/src/cameraManager.js index b2a3471..5420a8b 100644 --- a/frontend/src/cameraManager.js +++ b/frontend/src/cameraManager.js @@ -1,7 +1,5 @@ -// Shared camera stream manager used by any component that needs webcam access -// (WebcamFileSource, Scan). Centralizing this means every consumer gets device -// enumeration, preferred-camera memory, stream reuse and disconnect/reconnect -// handling for free instead of re-implementing it per component. +// Shared camera stream manager for webcam access (WebcamFileSource, Scan) - centralizes device +// enumeration, preferred-camera memory, stream reuse, and disconnect/reconnect handling. class CameraManager { constructor() { this.availableCameras = []; diff --git a/frontend/src/components/AuthenticatedImage.vue b/frontend/src/components/AuthenticatedImage.vue index 4aae7d5..75bec1f 100644 --- a/frontend/src/components/AuthenticatedImage.vue +++ b/frontend/src/components/AuthenticatedImage.vue @@ -73,9 +73,8 @@ export default { return; } try { - // Cached by src alone (content is hash-addressed and immutable - see - // fileCache.js) so every AuthenticatedImage instance showing the same - // file, across the whole app, shares one fetch and one decoded blob. + // Cached by src alone: content is hash-addressed and immutable (see fileCache.js), + // so every instance showing the same file shares one fetch and blob. const url = await fileCache.get(this.src, async () => { this.servers = await this.getFriendServers({username: this.owner}); const response = await this.servers.getRaw(this.signAuth, this.src); diff --git a/frontend/src/components/LabelLayoutPreview.vue b/frontend/src/components/LabelLayoutPreview.vue index 17a6645..5faf433 100644 --- a/frontend/src/components/LabelLayoutPreview.vue +++ b/frontend/src/components/LabelLayoutPreview.vue @@ -6,9 +6,9 @@
+ :class="{'template-option-disabled': !isSelectable(t)}" + :title="unavailableReason(t)" + role="button" @click="isSelectable(t) && $emit('input', t.id)"> @@ -24,8 +24,7 @@