From f729efb9d48f4312b9cb4ea9a14c8342807ab7ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laura=20Kl=C3=BCnder?= Date: Mon, 26 Aug 2024 14:27:25 +0200 Subject: [PATCH] in-between state, we now need to fix geometry field serialization --- src/c3nav/editor/models/changeset.py | 17 +---- src/c3nav/editor/operations.py | 92 +++++++++++++++++++++-- src/c3nav/editor/overlay.py | 11 ++- src/c3nav/editor/views/base.py | 47 ++++++++++-- src/c3nav/editor/views/changes.py | 36 ++++----- src/c3nav/editor/views/edit.py | 105 +++++++++++++-------------- 6 files changed, 205 insertions(+), 103 deletions(-) diff --git a/src/c3nav/editor/models/changeset.py b/src/c3nav/editor/models/changeset.py index 998eeb81..ef70528e 100644 --- a/src/c3nav/editor/models/changeset.py +++ b/src/c3nav/editor/models/changeset.py @@ -125,6 +125,7 @@ class ChangeSet(models.Model): Wrap Objects """ def fill_changes_cache(self): + return """ Get all changed objects and fill this ChangeSet's changes cache. Only executable once, if something is changed later the cache will be automatically updated. @@ -213,24 +214,9 @@ class ChangeSet(models.Model): @contextmanager def lock_to_edit(self, request=None): with transaction.atomic(): - user = request.user if request is not None and request.user.is_authenticated else None if self.pk is not None: changeset = ChangeSet.objects.select_for_update().get(pk=self.pk) - - self._object_changed = False yield changeset - if self._object_changed: - update = changeset.updates.create(user=user, objects_changed=True) - changeset.last_update = update - changeset.last_change = update - changeset.save() - elif self.direct_editing: - with MapUpdate.lock(): - changed_geometries.reset() - ChangeSet.objects_changed_count = 0 - yield self - if ChangeSet.objects_changed_count: - MapUpdate.objects.create(user=user, type='direct_edit') else: yield self @@ -460,6 +446,7 @@ class ChangeSet(models.Model): if self.direct_editing: return _('Direct editing active') return _('No objects changed') + return 'something was changed' # todo: make this nice again return (ngettext_lazy('%(num)d object changed', '%(num)d objects changed', 'num') % {'num': self.changed_objects_count}) diff --git a/src/c3nav/editor/operations.py b/src/c3nav/editor/operations.py index 5af677f6..9773353f 100644 --- a/src/c3nav/editor/operations.py +++ b/src/c3nav/editor/operations.py @@ -1,7 +1,12 @@ +import copy import datetime +import json +from dataclasses import dataclass from typing import TypeAlias, Any, Annotated, Literal, Union -from django.db import models +from django.apps import apps +from django.core import serializers +from django.db.models import Model from django.utils import timezone from pydantic.config import ConfigDict from pydantic.fields import Field @@ -18,7 +23,7 @@ class ObjectReference(BaseSchema): id: int @classmethod - def from_instance(cls, instance: models.Model): + def from_instance(cls, instance: Model): """ This method will not convert the ID yet! """ @@ -29,20 +34,46 @@ class BaseOperation(BaseSchema): obj: ObjectReference datetime: Annotated[datetime.datetime, Field(default_factory=timezone.now)] + def apply(self, values: FieldValuesDict, instance: Model) -> Model: + raise NotImplementedError + class CreateObjectOperation(BaseOperation): type: Literal["create"] = "create" fields: FieldValuesDict + def apply_create(self) -> Model: + instance = list(serializers.deserialize("json", json.dumps([{ + "model": f"mapdata.{self.obj.model}", + "pk": self.obj.id, + "fields": self.fields, + }])))[0] + instance.save(save_m2m=False) + return instance.object + class UpdateObjectOperation(BaseOperation): type: Literal["update"] = "update" fields: FieldValuesDict + def apply(self, values: FieldValuesDict, instance: Model) -> Model: + values.update(self.fields) + instance = list(serializers.deserialize("json", json.dumps([{ + "model": f"mapdata.{self.obj.model}", + "pk": self.obj.id, + "fields": self.fields, + }])))[0] + instance.save(save_m2m=False) + return instance.object + class DeleteObjectOperation(BaseOperation): type: Literal["delete"] = "delete" + def apply(self, values: FieldValuesDict, instance: Model) -> Model: + instance.delete() + return instance + class UpdateManyToManyOperation(BaseOperation): type: Literal["m2m_add"] = "m2m_update" @@ -50,11 +81,23 @@ class UpdateManyToManyOperation(BaseOperation): add_values: set[int] = set() remove_values: set[int] = set() + def apply(self, values: FieldValuesDict, instance: Model) -> Model: + values[self.field] = sorted((set(values[self.field]) | self.add_values) - self.remove_values) + field_manager = getattr(instance, self.field) + field_manager.add(*self.add_values) + field_manager.remove(*self.remove_values) + return instance + class ClearManyToManyOperation(BaseOperation): type: Literal["m2m_clear"] = "m2m_clear" field: str + def apply(self, values: FieldValuesDict, instance: Model) -> Model: + values[self.field] = [] + getattr(instance, self.field).clear() + return instance + DatabaseOperation = Annotated[ Union[ @@ -69,6 +112,45 @@ DatabaseOperation = Annotated[ class CollectedChanges(BaseSchema): - prev_reprs: dict[ObjectReference, str] = {} - prev_values: dict[ObjectReference, FieldValuesDict] = {} - operations: list[DatabaseOperation] = [] \ No newline at end of file + prev_reprs: dict[str, dict[int, str]] = {} + prev_values: dict[str, dict[int, FieldValuesDict]] = {} + operations: list[DatabaseOperation] = [] + + def prefetch(self) -> "CollectedChangesPrefetch": + ids_to_query: dict[str, set[int]] = {model_name: set(val.keys()) + for model_name, val in self.prev_values.items()} + + instances: dict[ObjectReference, Model] = {} + for model_name, ids in ids_to_query.items(): + model = apps.get_model("mapdata", model_name) + instances.update(dict((ObjectReference(model=model_name, id=instance.pk), instance) + for instance in model.objects.filter(pk__in=ids))) + + return CollectedChangesPrefetch(changes=self, instances=instances) + + +@dataclass +class CollectedChangesPrefetch: + changes: CollectedChanges + instances: dict[ObjectReference, Model] + + def apply(self): + prev_values = copy.deepcopy(self.changes.prev_values) + for operation in self.changes.operations: + if isinstance(operation, CreateObjectOperation): + self.instances[operation.obj] = operation.apply_create() + else: + in_prev_values = operation.obj.id in prev_values.get(operation.obj.model, {}) + if not in_prev_values: + print('WARN WARN WARN') + values = prev_values.setdefault(operation.obj.model, {}).setdefault(operation.obj.id, {}) + try: + instance = self.instances[operation.obj] + except KeyError: + if not in_prev_values: + instance = apps.get_model("mapdata", operation.obj.model).filter(pk=operation.obj.id).first() + else: + instance = None + if instance is not None: + operation.apply(values=values, instance=instance) + diff --git a/src/c3nav/editor/overlay.py b/src/c3nav/editor/overlay.py index 3115fe37..f4831126 100644 --- a/src/c3nav/editor/overlay.py +++ b/src/c3nav/editor/overlay.py @@ -40,8 +40,8 @@ class DatabaseOverlayManager: try: with transaction.atomic(): manager = DatabaseOverlayManager(changes) + manager.changes.prefetch().apply() overlay_state.manager = manager - # todo: apply changes so far yield manager if not commit: raise InterceptAbortTransaction @@ -50,6 +50,9 @@ class DatabaseOverlayManager: finally: overlay_state.manager = None + def save_new_operations(self): + self.changes.operations.extend(self.new_operations) + @staticmethod def get_model_field_values(instance: Model) -> FieldValuesDict: return json.loads(serializers.serialize("json", [instance]))[0]["fields"] @@ -59,8 +62,8 @@ class DatabaseOverlayManager: pre_change_values = self.pre_change_values.pop(ref, None) if pre_change_values: - self.changes.prev_values[ref] = pre_change_values - self.changes.prev_reprs[ref] = str(instance) + self.changes.prev_values.setdefault(ref.model, {})[ref.id] = pre_change_values + self.changes.prev_reprs.setdefault(ref.model, {})[ref.id] = str(instance) return ref @@ -68,7 +71,7 @@ class DatabaseOverlayManager: if instance.pk is None: return ref = ObjectReference.from_instance(instance) - if ref not in self.pre_change_values and ref not in self.changes.prev_values: + if ref not in self.pre_change_values and ref.id not in self.changes.prev_values.get(ref.model, {}): self.pre_change_values[ref] = self.get_model_field_values( instance._meta.model.objects.get(pk=instance.pk) ) diff --git a/src/c3nav/editor/views/base.py b/src/c3nav/editor/views/base.py index be750501..e8cc94f1 100644 --- a/src/c3nav/editor/views/base.py +++ b/src/c3nav/editor/views/base.py @@ -1,5 +1,6 @@ from abc import ABC, abstractmethod from collections import OrderedDict +from contextlib import contextmanager from functools import wraps from typing import Optional @@ -17,19 +18,52 @@ from django.utils.translation import gettext_lazy as _ from c3nav.editor.models import ChangeSet from c3nav.editor.overlay import DatabaseOverlayManager +from c3nav.mapdata.models import MapUpdate from c3nav.mapdata.models.access import AccessPermission from c3nav.mapdata.models.base import SerializableMixin +from c3nav.mapdata.utils.cache.changes import changed_geometries from c3nav.mapdata.utils.user import can_access_editor +@contextmanager +def maybe_lock_changeset_to_edit(request): + if request.changeset.pk: + with request.changeset.lock_to_edit(request=request) as changeset: + request.changeset = changeset + yield + else: + yield + + def accesses_mapdata(func): @wraps(func) def wrapped(request, *args, **kwargs): - changes = None if request.changeset.direct_editing is None else request.changeset.changes - with DatabaseOverlayManager.enable(changes, commit=request.changeset.direct_editing) as manager: - result = func(request, *args, **kwargs) - print("operations", manager.new_operations) - return result + writable_method = request.method in ("POST", "PUT") + + if request.changeset.direct_editing: + with MapUpdate.lock(): + changed_geometries.reset() + with DatabaseOverlayManager.enable(changes=None, commit=writable_method) as manager: + result = func(request, *args, **kwargs) + if manager.new_operations: + if writable_method: + MapUpdate.objects.create(user=request.user, type='direct_edit') + else: + raise ValueError # todo: good error message, but this shouldn't happen + else: + with maybe_lock_changeset_to_edit(request=request): + with DatabaseOverlayManager.enable(changes=request.changeset.changes, commit=False) as manager: + result = func(request, *args, **kwargs) + if manager.new_operations: + manager.save_new_operations() + print(request.changeset.changes) + print('saving to changeset!!') + request.changeset.save() + update = request.changeset.updates.create(user=request.user, objects_changed=True) + request.changeset.last_update = update + request.changeset.last_change = update + request.changeset.save() + return result return wrapped @@ -48,7 +82,8 @@ def sidebar_view(func=None, select_related=None, api_hybrid=False): if not can_access_editor(request): raise PermissionDenied - request.changeset = ChangeSet.get_for_request(request, select_related) + if getattr(request, "changeset", None) is None: + request.changeset = ChangeSet.get_for_request(request, select_related) if api: request.is_delete = request.method == 'DELETE' diff --git a/src/c3nav/editor/views/changes.py b/src/c3nav/editor/views/changes.py index 11378eef..c46935ed 100644 --- a/src/c3nav/editor/views/changes.py +++ b/src/c3nav/editor/views/changes.py @@ -38,24 +38,24 @@ def changeset_detail(request, pk): if request.method == 'POST': restore = request.POST.get('restore') if restore and restore.isdigit(): - with changeset.lock_to_edit(request) as changeset: - if changeset.can_edit(request): - try: - changed_object = changeset.changed_objects_set.get(pk=restore) - except Exception: - pass - else: - try: - changed_object.restore() - messages.success(request, _('Object has been successfully restored.')) - except PermissionError: - messages.error(request, _('You cannot restore this object, because it depends on ' - 'a deleted object or it would violate a unique contraint.')) - - else: - messages.error(request, _('You can not edit changes on this change set.')) - - return redirect(reverse('editor.changesets.detail', kwargs={'pk': changeset.pk})) + raise NotImplementedError # todo: restore (no pun intended) this feature + # if request.changeset.can_edit(request): + # try: + # changed_object = changeset.changed_objects_set.get(pk=restore) + # except Exception: + # pass + # else: + # try: + # changed_object.restore() + # messages.success(request, _('Object has been successfully restored.')) + # except PermissionError: + # messages.error(request, _('You cannot restore this object, because it depends on ' + # 'a deleted object or it would violate a unique contraint.')) + # + # else: + # messages.error(request, _('You can not edit changes on this change set.')) + # + # return redirect(reverse('editor.changesets.detail', kwargs={'pk': changeset.pk})) elif request.POST.get('activate') == '1': with changeset.lock_to_edit(request) as changeset: diff --git a/src/c3nav/editor/views/edit.py b/src/c3nav/editor/views/edit.py index 426b37b5..9f06fe57 100644 --- a/src/c3nav/editor/views/edit.py +++ b/src/c3nav/editor/views/edit.py @@ -128,7 +128,7 @@ def space_detail(request, level, pk): def get_changeset_exceeded(request): - return request.user_permissions.max_changeset_changes <= request.changeset.changed_objects_count + return request.user_permissions.max_changeset_changes <= len(request.changeset.changes.operations) @etag(editor_etag_func) @@ -315,15 +315,14 @@ def edit(request, pk=None, model=None, level=None, space=None, on_top_of=None, e ) if request.POST.get('delete_confirm') == '1' or delete: - with request.changeset.lock_to_edit(request) as changeset: - if changeset.can_edit(request): - obj.delete() - else: - return APIHybridMessageRedirectResponse( - level='error', - message=_('You can not edit changes on this changeset.'), - redirect_to=request.path, status_code=403, - ) + if request.changeset.can_edit(request): # todo: move this somewhere else + obj.delete() + else: + return APIHybridMessageRedirectResponse( + level='error', + message=_('You can not edit changes on this changeset.'), + redirect_to=request.path, status_code=403, + ) if model == Level: if obj.on_top_of_id is not None: @@ -360,28 +359,27 @@ def edit(request, pk=None, model=None, level=None, space=None, on_top_of=None, e if on_top_of is not None: obj.on_top_of = on_top_of - with request.changeset.lock_to_edit(request) as changeset: - if changeset.can_edit(request): - try: - obj.save() - except IntegrityError: - error = APIHybridError(status_code=400, message=_('Duplicate entry.')) - else: - if form.redirect_slugs is not None: - for slug in form.add_redirect_slugs: - obj.redirects.create(slug=slug) - - for slug in form.remove_redirect_slugs: - obj.redirects.filter(slug=slug).delete() - - form.save_m2m() - return APIHybridMessageRedirectResponse( - level='success', - message=_('Object was successfully saved.'), - redirect_to=ctx['back_url'] - ) + if request.changeset.can_edit(request): # todo: move this somewhere else + try: + obj.save() + except IntegrityError: + error = APIHybridError(status_code=400, message=_('Duplicate entry.')) else: - error = APIHybridError(status_code=403, message=_('You can not edit changes on this changeset.')) + if form.redirect_slugs is not None: + for slug in form.add_redirect_slugs: + obj.redirects.create(slug=slug) + + for slug in form.remove_redirect_slugs: + obj.redirects.filter(slug=slug).delete() + + form.save_m2m() + return APIHybridMessageRedirectResponse( + level='success', + message=_('Object was successfully saved.'), + redirect_to=ctx['back_url'] + ) + else: + error = APIHybridError(status_code=403, message=_('You can not edit changes on this changeset.')) else: form = get_editor_form(model)(instance=obj, request=request, space_id=space_id, @@ -640,14 +638,13 @@ def graph_edit(request, level=None, space=None): return redirect(request.path) if request.POST.get('delete_confirm') == '1': - with request.changeset.lock_to_edit(request) as changeset: - if changeset.can_edit(request): - node.edges_from_here.all().delete() - node.edges_to_here.all().delete() - node.delete() - else: - messages.error(request, _('You can not edit changes on this changeset.')) - return redirect(request.path) + if request.changeset.can_edit(request): # todo: move this somewhere else + node.edges_from_here.all().delete() + node.edges_to_here.all().delete() + node.delete() + else: + messages.error(request, _('You can not edit changes on this changeset.')) + return redirect(request.path) messages.success(request, _('Graph Node was successfully deleted.')) return redirect(request.path) return render(request, 'editor/delete.html', { @@ -677,13 +674,12 @@ def graph_edit(request, level=None, space=None): active_node = None set_active_node = True else: - with request.changeset.lock_to_edit(request) as changeset: - if changeset.can_edit(request): - connect_nodes(request, active_node, clicked_node, edge_settings_form) - active_node = clicked_node if edge_settings_form.cleaned_data['activate_next'] else None - set_active_node = True - else: - messages.error(request, _('You can not edit changes on this changeset.')) + if request.changeset.can_edit(request): # todo: move this somewhere else + connect_nodes(request, active_node, clicked_node, edge_settings_form) + active_node = clicked_node if edge_settings_form.cleaned_data['activate_next'] else None + set_active_node = True + else: + messages.error(request, _('You can not edit changes on this changeset.')) elif (clicked_node is None and clicked_position is not None and active_node is None and space.geometry.contains(clicked_position)): @@ -692,16 +688,15 @@ def graph_edit(request, level=None, space=None): messages.error(request, _('You can not add graph nodes because your changeset is full.')) return redirect(request.path) - with request.changeset.lock_to_edit(request) as changeset: - if changeset.can_edit(request): - node = GraphNode(space=space, geometry=clicked_position) - node.save() - messages.success(request, _('New graph node created.')) + if request.changeset.can_edit(request): # todo: move this somewhere else + node = GraphNode(space=space, geometry=clicked_position) + node.save() + messages.success(request, _('New graph node created.')) - active_node = None - set_active_node = True - else: - messages.error(request, _('You can not edit changes on this changeset.')) + active_node = None + set_active_node = True + else: + messages.error(request, _('You can not edit changes on this changeset.')) if set_active_node: connections = {}