From ee13b913999f9e9fd7ac18cb00874baf8f580267 Mon Sep 17 00:00:00 2001 From: Erudition <3915447+Erudition@users.noreply.github.com> Date: Mon, 18 Nov 2024 14:39:31 -0600 Subject: [PATCH] Clarify reversions as deletions --- elm/Replicated/Codec.elm | 4 +- elm/Replicated/Node/Node.elm | 81 +++++++++++++++++++++++++++++++----- elm/Replicated/Object.elm | 6 +-- elm/Replicated/Op/Op.elm | 6 +-- elm/Replicated/Op/OpID.elm | 28 ++++++------- 5 files changed, 93 insertions(+), 32 deletions(-) diff --git a/elm/Replicated/Codec.elm b/elm/Replicated/Codec.elm index 46f33fd0..83957028 100644 --- a/elm/Replicated/Codec.elm +++ b/elm/Replicated/Codec.elm @@ -937,13 +937,13 @@ id = Just node -> case Node.lookupObject node opID of - Err _ -> + Nothing -> Log.crashInDev ("Un-serializing an ID " ++ asString ++ " but I couldn't find the object referenced in the node!") ID.fromPointer (ExistingObjectPointer (Change.ExistingID "error" opID)) - Ok ( reducerID, objectID ) -> + Just ( reducerID, objectID ) -> -- TODO should we use the OpID instead? For versioning? -- Or is this better to switch to canonical ObjectIDs ID.fromPointer (ExistingObjectPointer (Change.ExistingID reducerID objectID)) diff --git a/elm/Replicated/Node/Node.elm b/elm/Replicated/Node/Node.elm index 0b8a4c62..c651748c 100644 --- a/elm/Replicated/Node/Node.elm +++ b/elm/Replicated/Node/Node.elm @@ -10,6 +10,7 @@ import List.Nonempty as Nonempty exposing (Nonempty(..)) import Log import Maybe.Extra import Parser.Advanced as Parser +import Set.Any as AnySet exposing (AnySet) import Replicated.Change as Change exposing (Change, ChangeSet(..), ComplexAtom, PendingID, Pointer(..), pendingIDToString) import Replicated.Change.Location as Location exposing (Location) import Replicated.Identifier exposing (..) @@ -264,16 +265,20 @@ updateNodeWithChunk chunk old = Just firstOpenOp -> case ( firstOpenOp.objectSpecified, firstOpenOp.reducerSpecified, firstOpenOp.reference ) of - ( Just explicitReducer, Just explicitObject, _ ) -> + ( Just explicitObject, Just explicitReducer, _ ) -> -- closed ops - reducer and objectID are explicit - Ok ( explicitObject, explicitReducer ) + Ok ( explicitReducer, explicitObject ) ( _, _, Op.ReducerReference reducerID ) -> -- It's a header / creation op, no need to lookup Ok ( reducerID, firstOpenOp.opID ) ( _, _, Op.OpReference referencedOpID ) -> - lookupObject old.node referencedOpID + case lookupObject old.node referencedOpID of + Just foundBoth -> + Ok foundBoth + Nothing -> + Err (UnknownReference referencedOpID) closedOpListResult = case deduceChunkReducerAndObject of @@ -311,15 +316,15 @@ closeOp deducedReducer deducedObject openOp = First we compare against object creation IDs, then the stored "last seen" IDs, since it will usually be that. Finally, we check all other op IDs. -} -lookupObject : Node -> OpID -> Result OpImportWarning ( ReducerID, ObjectID ) +lookupObject : Node -> OpID -> Maybe ( ReducerID, ObjectID ) lookupObject node opIDToFind = case AnyDict.get opIDToFind node.ops of -- will even find objects by middle ops (version references) Just foundOp -> - Ok ( Op.reducer foundOp, Op.object foundOp ) + Just ( Op.reducer foundOp, Op.object foundOp ) Nothing -> - Err (UnknownReference opIDToFind) + Nothing {-| Quick way to see how many recognized objects are in the Node. @@ -479,6 +484,62 @@ getReversibleOps ops = in List.filterMap getCreationIDs ops +{-| Find all Ops that use the given Op as a reference. +TODO just index the OpDb by reference +-} +findAllReferencesToOp : Node -> OpID -> List Op +findAllReferencesToOp node opID = + AnyDict.filter (\_ op -> Op.opIDFromReference (Op.reference op) == Just opID) node.ops + |> AnyDict.values + + +{-| Given a list of OpIDs representing the original change, find the Ops of their latest reversion. + +TODO: once OpDb is indexed by object and reference, eliminate the recursive search. +-} +findFinalOpsToRevert : Node -> Change.UndoData -> List Op +findFinalOpsToRevert node opIDsToRevert = + let + findFinalReversionOp earlierOpID = + let + earlierOpMaybe = + -- TODO ideally we don't have to fetch the op itself for this. + AnyDict.get earlierOpID node.ops + + earlierOpPatternMaybe = + Maybe.map (Op.pattern) earlierOpMaybe + + -- if the op is normal, the next reversion will be a deletion op. + -- if the op is a deletion already, the next reversion (undeletion) will look like a normal opID. + opPatternToLookFor = + case earlierOpPatternMaybe of + Just Op.DeletionOp -> + Op.UnDeletionOp + + Just Op.UnDeletionOp -> + Op.DeletionOp + + _ -> + Op.NormalOp + + allOpsReferringToEarlierOp = + findAllReferencesToOp node earlierOpID + in + case List.filter (\op -> Op.pattern op == opPatternToLookFor ) allOpsReferringToEarlierOp of + [] -> + -- Looks like this op was never reverted, so it's the final op + earlierOpMaybe + + [foundReversionOp] -> + -- the earlier op has been reverted, repeat the process with the newfound reversion + findFinalReversionOp (Op.id foundReversionOp) + + firstOpFound :: moreFound -> + Log.crashInDev "When trying to find reversions of an op, I found multiple..." (Just firstOpFound) + in + List.filterMap findFinalReversionOp (AnySet.toList opIDsToRevert) + + {-| Collects info on what ObjectIDs map back to what placeholder IDs from before they were initialized. In case we want to reference the new object same-frame. Use with Change.pendingIDToString @@ -669,7 +730,7 @@ objectChangeChunkToOps node pointer objectChanges ( inCounter, inMapping, inChun stampChunkOps ( stampInCounter, opIDToReference ) givenUCO = let ( newID, stampOutCounter ) = - OpID.generate stampInCounter node.identity givenUCO.reversion + OpID.generate stampInCounter node.identity givenUCO.deletion stampedOp = Op.create reducerID objectID newID (Op.OpReference <| Maybe.withDefault opIDToReference givenUCO.reference) givenUCO.payload @@ -693,7 +754,7 @@ objectChangeChunkToOps node pointer objectChanges ( inCounter, inMapping, inChun type alias UnstampedChunkOp = - { reference : Maybe OpID, payload : Op.OpPayloadAtoms, reversion : Bool } + { reference : Maybe OpID, payload : Op.OpPayloadAtoms, deletion : Bool } {-| Get prerequisite ops for an (existing object) change if needed, then process the change into an UnstampedChunkOp, leaving out the other op fields to be added by the caller @@ -805,7 +866,7 @@ objectChangeToUnstampedOp node ( inCounter, inMapping ) objectChange = , thisUnstampedOp = { reference = reference , payload = piecesSoFar - , reversion = False + , deletion = False } } ) @@ -820,7 +881,7 @@ objectChangeToUnstampedOp node ( inCounter, inMapping ) objectChange = Change.RevertOp opIDToRevert -> ( ( inCounter, inMapping ) , { prerequisiteChunks = [] - , thisUnstampedOp = { reference = Just opIDToRevert, payload = [], reversion = not (OpID.isReversion opIDToRevert) } + , thisUnstampedOp = { reference = Just opIDToRevert, payload = [], deletion = not (OpID.isDeletion opIDToRevert) } } ) diff --git a/elm/Replicated/Object.elm b/elm/Replicated/Object.elm index 2521fb15..fe9252c1 100644 --- a/elm/Replicated/Object.elm +++ b/elm/Replicated/Object.elm @@ -80,7 +80,7 @@ applyOp opDict newOp ( oldObject, oldWarnings ) = -- op ref means it's an event op (or reversion) let ( newEventDict, newWarnings ) = - if OpID.isReversion (Op.id newOp) then + if OpID.isDeletion (Op.id newOp) then -- this op reverts a real event revertEventHelper ref oldObject.events opDict -- |> Debug.log ("Op " ++ OpID.toString (Op.id newOp) ++ " reverts op " ++ OpID.toString ref ++ " in object " ++ OpID.toString oldObject.creation ++ ". new event dict") @@ -111,7 +111,7 @@ applyOp opDict newOp ( oldObject, oldWarnings ) = -} revertEventHelper : OpID -> EventDict -> OpDict -> ( EventDict, List ObjectBuildWarning ) revertEventHelper opIDToRevert eventDict opDict = - case ( AnyDict.member opIDToRevert eventDict, OpID.isReversion opIDToRevert ) of + case ( AnyDict.member opIDToRevert eventDict, OpID.isDeletion opIDToRevert ) of ( True, _ ) -> -- found our reverted event. remove it! ( AnyDict.remove opIDToRevert eventDict, [] ) @@ -131,7 +131,7 @@ revertEventHelper opIDToRevert eventDict opDict = Op.OpReference thirdOpID -> -- does the second reversion op point to a third reversion op? - if OpID.isReversion thirdOpID then + if OpID.isDeletion thirdOpID then -- yup. start over with that one. revertEventHelper thirdOpID eventDict opDict diff --git a/elm/Replicated/Op/Op.elm b/elm/Replicated/Op/Op.elm index 1a9dfd75..672be22d 100644 --- a/elm/Replicated/Op/Op.elm +++ b/elm/Replicated/Op/Op.elm @@ -1,4 +1,4 @@ -module Replicated.Op.Op exposing (ClosedChunk, Context(..), FrameChunk, Op(..), OpPattern(..), OpPayloadAtom(..), OpPayloadAtoms, OpenTextOp, OpenTextRonFrame, Problem(..), ReducerID, Reference(..), RonFormat(..), atomToJsonValue, atomToRonString, closedChunksToFrameText, closedOpToString, contextStackToString, create, id, initObject, object, pattern, payload, payloadToJsonValue, problemToString, reducer, reference, ronParser) +module Replicated.Op.Op exposing (ClosedChunk, Context(..), FrameChunk, Op(..), OpPattern(..), OpPayloadAtom(..), OpPayloadAtoms, OpenTextOp, OpenTextRonFrame, Problem(..), ReducerID, Reference(..), RonFormat(..), atomToJsonValue, atomToRonString, closedChunksToFrameText, closedOpToString, contextStackToString, create, id, initObject, object, pattern, payload, payloadToJsonValue, problemToString, reducer, reference, ronParser, opIDFromReference) {-| Just Ops - already-happened events and such. Ignore Frames for now, they are "write batches" so once they're written they will slef-concatenate in the list of Ops. -} @@ -864,7 +864,7 @@ type OpPattern pattern : Op -> OpPattern pattern (Op opRecord) = - case ( OpID.isReversion opRecord.operationID, Maybe.map OpID.isReversion (opIDFromReference opRecord.reference) ) of + case ( OpID.isDeletion opRecord.operationID, Maybe.map OpID.isDeletion (opIDFromReference opRecord.reference) ) of ( False, Just False ) -> -- "+", "+" NormalOp @@ -983,7 +983,7 @@ closedOpToString format (Op op) = [ opID, ref ] CompressedOps (Just (Op previousOp)) -> - case ( OpID.isIncremental previousOp.operationID op.operationID && not (OpID.isReversion op.operationID), op.reference == OpReference previousOp.operationID ) of + case ( OpID.isIncremental previousOp.operationID op.operationID && not (OpID.isDeletion op.operationID), op.reference == OpReference previousOp.operationID ) of ( True, True ) -> [ emptyAtom, emptyAtom ] diff --git a/elm/Replicated/Op/OpID.elm b/elm/Replicated/Op/OpID.elm index a819ebfb..48de23df 100644 --- a/elm/Replicated/Op/OpID.elm +++ b/elm/Replicated/Op/OpID.elm @@ -1,4 +1,4 @@ -module Replicated.Op.OpID exposing (EventStamp, InCounter, ObjectID, ObjectIDString, ObjectVersion, OpID, OpIDSortable, OpIDString, OutCounter, exportCounter, firstCounterOfFrame, fromPrimitives, fromRonPointerString, fromSortable, fromString, fromStringForced, generate, getClock, getMoment, highestCounter, importCounter, isIncremental, isReversion, jsonDecoder, latest, nextGenCounter, nextOpInChain, parser, toInt, toRonPointerString, toSortablePrimitives, toString, unusedCounter) +module Replicated.Op.OpID exposing (EventStamp, InCounter, ObjectID, ObjectIDString, ObjectVersion, OpID, OpIDSortable, OpIDString, OutCounter, exportCounter, firstCounterOfFrame, fromPrimitives, fromRonPointerString, fromSortable, fromString, fromStringForced, generate, getClock, getMoment, highestCounter, importCounter, isIncremental, isDeletion, jsonDecoder, latest, nextGenCounter, nextOpInChain, parser, toInt, toRonPointerString, toSortablePrimitives, toString, unusedCounter) import Json.Decode as JD import Parser.Advanced as Parser exposing ((|.), (|=), Parser, float, spaces, succeed, symbol) @@ -41,13 +41,13 @@ type alias OpOrigin = type alias EventStamp = { clock : OpClock , origin : NodeID - , reversion : Bool + , deletion : Bool } -isReversion : OpID -> Bool -isReversion input = - (toStamp input).reversion +isDeletion : OpID -> Bool +isDeletion input = + (toStamp input).deletion highestCounter : NewOpCounter -> NewOpCounter -> NewOpCounter @@ -66,8 +66,8 @@ unusedCounter = generate : InCounter -> NodeID -> Bool -> ( OpID, OutCounter ) -generate (NewOpCounter counter) origin reversion = - ( fromStamp { clock = counter, origin = origin, reversion = reversion }, NewOpCounter (counter + 1) ) +generate (NewOpCounter counter) origin deletion = + ( fromStamp { clock = counter, origin = origin, deletion = deletion }, NewOpCounter (counter + 1) ) toString : OpID -> OpIDString @@ -78,11 +78,11 @@ toString (OpID string) = toSortablePrimitives : OpID -> OpIDSortable toSortablePrimitives opID = let - { clock, origin, reversion } = + { clock, origin, deletion } = toStamp opID tieBreaker = - if reversion then + if deletion then "-" ++ NodeID.toString origin else @@ -100,7 +100,7 @@ The origin could be used as a tie-breaker, modulating the clock number by some a toInt : OpID -> Int toInt opID = let - { clock, origin, reversion } = + { clock, origin, deletion } = toStamp opID in clock @@ -112,7 +112,7 @@ In production environments this number will be relatively close to the actual ti getClock : OpID -> OpClock getClock opID = let - { clock, origin, reversion } = + { clock, origin, deletion } = toStamp opID in clock @@ -131,10 +131,10 @@ fromSortable ( clock, rest ) = fromPrimitives : Int -> Bool -> String -> OpID -fromPrimitives clock reversion nodeName = +fromPrimitives clock deletion nodeName = OpID (String.fromInt clock - ++ (if reversion then + ++ (if deletion then "-" ++ nodeName else @@ -188,7 +188,7 @@ fromStamp eventStamp = String.fromInt eventStamp.clock separator = - if eventStamp.reversion then + if eventStamp.deletion then "-" else