From 34d79ee8df7bb396d9c1f78778f37dd29152af8f Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sun, 1 Nov 2015 07:03:47 +0100 Subject: [PATCH] tree-diff-application: unit test PASS well... this was quite a piece of work Added some documentation, but a complete documentation, preferably to the website, would be desirable, as would be a more complete test covering the negative corner cases --- src/lib/diff/tree-diff-application.hpp | 49 +++++++-- src/lib/diff/tree-diff.hpp | 74 +++++++++++-- tests/15library.tests | 5 + .../diff/diff-tree-application-test.cpp | 103 +++++++++++------- wiki/thinkPad.ichthyo.mm | 80 +++++++++++--- 5 files changed, 236 insertions(+), 75 deletions(-) diff --git a/src/lib/diff/tree-diff-application.hpp b/src/lib/diff/tree-diff-application.hpp index 119ed506c..055ac813e 100644 --- a/src/lib/diff/tree-diff-application.hpp +++ b/src/lib/diff/tree-diff-application.hpp @@ -40,16 +40,49 @@ ** a switch from scope to scope, which adds a lot of complexity. So the list diff ** application strategy can be seen as blueprint and demonstration of principles. ** - ** Another point in question is weather see the diff application as manipulating - ** a target data structure, or rather building a reshaped copy. The fact that - ** GenNode and Record are designed as immutable values seems to favour the latter, - ** yet the very reason to engage into building this diff framework was how to - ** handle partial updates within a expectedly very large UI model, reflecting + ** Another point in question is weather to treat the diff application as + ** manipulating a target data structure, or rather building a reshaped copy. + ** The fact that GenNode and Record are designed as immutable values seems to favour + ** the latter, yet the very reason to engage into building this diff framework was + ** how to handle partial updates within a expectedly very large UI model, reflecting ** the actual session model in Proc-Layer. So we end up working on a Mutator, ** which clearly signals we're going to reshape and re-rig the target data. ** - ** @see diff-list-application-test.cpp - ** @see VerbToken + ** \par State and nested scopes + ** Within the level of a single #Record, our tree diff language works similar to + ** the list diff (with the addition of the \c after(ID) verb, which is just a + ** shortcut to accept parts of the contents unaltered). But after possibly rearranging + ** the contents of an "object" (Record), the diff might open some of its child "objects" + ** by entering a nested scope. This is done with the \c mut(ID)....emu(ID) bracketing + ** construct. On the implementation side, this means we need to use a stack somehow. + ** The decision was to manage this stack explicitly, as a std::stack (heap memory). + ** Each entry on this stack is a "context frame" for list diff. Which makes the + ** tree diff applicator a highly statefull component. + ** + ** Even more so, since -- for \em performance reasons -- we try to alter the + ** tree shaped data structure \em in-place. We want to avoid the copy of possibly + ** deep sub-trees, when in the end we might be just rearranging their sequence order. + ** This design decision comes at a price tag though + ** - it subverts the immutable nature of \c Record and leads to + ** high dependency on data layout and implementation details of the latter. + ** This is at least prominently marked by working on a diff::Record::Mutator, + ** so the client has first to "open up" the otherwise immutable tree + ** - the actual list diff on each level works by first \em moving the entire + ** Record contents away into a temporary buffer and then \em moving them + ** back into new shape one by one. In case of a diff conflict (i.e. a + ** mismatch between the actual data structure and the assumptions made + ** for the diff message on the sender / generator side), an exception + ** is thrown, leaving the client with a possibly corrupted tree, where + ** parts might even still be stashed away in the temporary buffer, + ** and thus be lost. + ** We consider this unfortunate, yet justified by the very nature of applying a diff. + ** When the user needs safety or transactional behaviour, a deep copy should be made + ** before attaching the #DiffApplicator + ** + ** @see DiffTreeApplication_test + ** @see DiffListApplication_test + ** @see GenNodeBasic_test + ** @see tree-diff.hpp ** */ @@ -169,7 +202,7 @@ namespace diff{ throw error::State(_Fmt("Unable locate position 'after(%s)'") % elm.idi , LUMIERA_ERROR_DIFF_CONFLICT); } - + void __expect_valid_parent_scope (GenNode::ID const& idi) { diff --git a/src/lib/diff/tree-diff.hpp b/src/lib/diff/tree-diff.hpp index 6771d8cf4..f26738f2b 100644 --- a/src/lib/diff/tree-diff.hpp +++ b/src/lib/diff/tree-diff.hpp @@ -27,12 +27,29 @@ ** this building block defines the set of operations to express both structural ** and content changes in a given data structure. ** - ** @todo UNIMPLEMENTED as of 12/14 + ** This »tree diff language« does not rely on any concrete data structure or layout, + ** just on some general assumptions regarding the ordering and structure of the data. + ** - top level is a root record + ** - a record has a type, a collection of named attributes, and a collection of children + ** - all elements within a record are conceived as elements in ordered sequence, with the + ** attributes first, followed by the children. The end of the attribute scope is given + ** by the the appearance of the first unnamed entry, i.e the first child. + ** - the individual elements in these sequences have a distinguishable identity and + ** optionally a name (a named element is an attribute). + ** - moreover, the elements carry a typed payload data element, which possibly is + ** a \em nested record ("nested child object"). + ** - the typing of the elements is outside the scope of the diff language; it is + ** assumed that the receiver knows what types to expect and how to deal with them. + ** - only nested records may be \em mutated by the diff. All other elements can + ** only be inserted, moved or deleted (like elements in list diff) + ** By implementing the #TreeDiffInterpreter interface (visitor), a concrete usage + ** can receive a diff description and possibly apply it to suitable target data. ** ** @see diff-language.cpp - ** @see diff-tree-application-test.cpp - ** @see tree-diff.cpp - ** @see GenNode + ** @see tree-diff-application.cpp + ** @see DiffTreeApplication_test + ** @see list-diff.cpp + ** @see diff::GenNode ** */ @@ -54,9 +71,52 @@ namespace diff{ * Interpreter interface to define the operations ("verbs"), * which describe differences or changes in hierarchical data structure. * The meaning of the verbs is as follows: - * - \c TODO - * - * @todo to be defined + * - \c ins prompts to insert the given argument element at the \em current + * processing position into the target sequence. This operation + * allows to inject new data + * - \c del requires to delete the \em next element at \em current position. + * For sake of verification, the ID of the argument payload is + * required to match the ID of the element about to be discarded. + * - \c pick just accepts the \em next element at \em current position into + * the resulting altered sequence. Again, the ID of the argument + * has to match the ID of the element to be picked, for sake + * of verification. + * - \c find effect a re-ordering of the target scope contents: it requires + * to \em search for the (next respective single occurrence of the) + * given element further down into the remainder of the current + * record scope (but not into nested child scopes). The designated + * element is to be retrieved and inserted as the next element + * at current position. + * - \c skip processing hint, emitted at the position where an element + * previously extracted by a \c find verb happened to sit within + * the old order. This allows an optimising implementation to “fetch” + * a copy and just drop or skip the original, thereby avoiding to + * shift any other elements. + * - \c after shortcut to \c pick existing elements up to the designated point. + * As a special notation, \c after(Ref::ATTRIBUTES) allows to fast forward + * to the first child element, while \c after(Ref::END) means to accept + * all of the existing data contents as-is (possibly to append further + * elements after that point. + * - \c mut bracketing construct to open a nested sub scope. The element + * designated by the ID of the argument needs to be a #Record + * ("nested child object"). Moreover, this element must have been + * mentioned with the preceding diff verbs at that level, which means + * that the element as such must already be present in the altered + * target structure. The \c mut(ID) verb then opens this nested + * record for diff handling, and all subsequent diff verbs are to be + * interpreted relative to this scope, until the corresponding + * \c emu(ID) verb is encountered. As a special notation, right + * after handling an element with the list diff verbs (i.e. \c ins + * or \c pick or \c find), it is allowed immediately to open the + * nested scope with \c mut(Ref::THIS) -- which circumvents the + * problem that it is sometimes difficult to know the precise ID, + * especially when hand-writing a diff to populate a data structure. + * - \c emu bracketing construct and counterpart to \c mut(ID). This verb + * must be given precisely at the end of the nested scope (it is + * not allowed to "return" from the middle of a scope, for sake + * of sanity). At this point, ths child scope is left and the + * parent scope with all existing diff state is popped from an + * internal stack */ class TreeDiffInterpreter { diff --git a/tests/15library.tests b/tests/15library.tests index 5a66af384..6ffee7ec3 100644 --- a/tests/15library.tests +++ b/tests/15library.tests @@ -158,6 +158,11 @@ return: 0 END +TEST "Diff: reshape a tree data structure through diff" DiffTreeApplication_test < is populated + * with a type-ID, three named attribute values, three child values + * and a nested child-Record. + * - the second step demonstrates various diff language constructs + * to alter, reshape and mutate this data structure + * After applying those two diff sequences, we verify the data + * is indeed in the expected shape. + * @remarks to follow this test, you should be familiar both with our + * \link diff::Record generic data record \endlink, as well as with + * the \link diff::GenNode variant data node \endlink. The key point + * to note is the usage of Record elements as payload within GenNode, + * which allows to represent tree shaped object like data structures. + * @see GenericRecordRepresentation_test + * @see GenNodeBasic_test + * @see DiffListApplication_test + * @see diff-tree-application.hpp + * @see tree-diff.hpp */ class DiffTreeApplication_test : public Test @@ -115,7 +131,7 @@ namespace test{ , pick(Ref::CHILD) // pick a child anonymously , mut(Ref::THIS) // mutate the current element (the one just picked) , ins(ATTRIB3) - , ins(ATTRIB_NODE) + , ins(ATTRIB_NODE) // attributes can also be nested objects , find(CHILD_A) , del(CHILD_B) , ins(CHILD_NODE) @@ -125,7 +141,7 @@ namespace test{ , ins(TYPE_Y) , ins(ATTRIB2) , emu(CHILD_NODE) - , mut(ATTRIB_NODE) + , mut(ATTRIB_NODE) // mutation can be out-of order, target found by ID , ins(CHILD_A) , ins(CHILD_A) , ins(CHILD_A) @@ -141,41 +157,44 @@ namespace test{ Rec::Mutator target; Rec& subject = target; DiffApplicator application(target); + + // Part I : apply diff to populate application.consume(populationDiff()); - CHECK (!isnil (subject)); // nonempty -- content has been added - CHECK ("X" == subject.getType()); // type was set to "X" - CHECK (1 == subject.get("α").data.get()); // has gotten our int attribute "α" - CHECK (2L == subject.get("β").data.get()); // ... the long attribute "β" - CHECK (3.45 == subject.get("γ").data.get()); // ... and double attribute "γ" - auto scope = subject.scope(); // look into the scope contents... - CHECK ( *scope == CHILD_A); // there is CHILD_A - CHECK (*++scope == CHILD_T); // followed by a copy of CHILD_T - CHECK (*++scope == CHILD_T); // and another copy of CHILD_T - CHECK (*++scope == MakeRec().appendChild(CHILD_B) // and there is a nested Record - .appendChild(CHILD_A) // with CHILD_B - .genNode(SUB_NODE.idi.getSym())); // and CHILD_A - CHECK (isnil(++scope)); // thats all -- no more children + CHECK (!isnil (subject)); // nonempty -- content has been added + CHECK ("X" == subject.getType()); // type was set to "X" + CHECK (1 == subject.get("α").data.get()); // has gotten our int attribute "α" + CHECK (2L == subject.get("β").data.get()); // ... the long attribute "β" + CHECK (3.45 == subject.get("γ").data.get()); // ... and double attribute "γ" + auto scope = subject.scope(); // look into the scope contents... + CHECK ( *scope == CHILD_A); // there is CHILD_A + CHECK (*++scope == CHILD_T); // followed by a copy of CHILD_T + CHECK (*++scope == CHILD_T); // and another copy of CHILD_T + CHECK (*++scope == MakeRec().appendChild(CHILD_B) // and there is a nested Record + .appendChild(CHILD_A) // with CHILD_B + .genNode(SUB_NODE.idi.getSym())); // and CHILD_A + CHECK (isnil(++scope)); // thats all -- no more children - application.consume(mutationDiff()); - CHECK (join (subject.keys()) == "α, β, γ"); // the attributes weren't altered - scope = subject.scope(); // but the scope was reordered - CHECK ( *scope == CHILD_T); // CHILD_T - CHECK (*++scope == CHILD_A); // CHILD_A - Rec nested = (++scope)->data.get(); // and our nested Record, which too has been altered: - CHECK (nested.get("γ").data.get() == 3.45); // it carries now an attribute "δ", which is again - CHECK (nested.get("δ").data.get() == MakeRec().appendChild(CHILD_A) // a nested Record with three children CHILD_A - .appendChild(CHILD_A) // - .appendChild(CHILD_A) // - .genNode("δ")); // - auto subScope = nested.scope(); // and within the nested sub-scope we find - CHECK ( *subScope == CHILD_A); // CHILD_A - CHECK (*++subScope == MakeRec().type("Y") // a yet-again nested sub-Record of type "Y" - .set("β", 2L ) // with just an attribute "β" == 2L - .genNode(CHILD_NODE.idi.getSym())); // (and an empty child scope) - CHECK (*++subScope == CHILD_T); // followed by another copy of CHILD_T - CHECK (isnil (++subScope)); // - CHECK (isnil (++scope)); // and nothing beyond that. + // Part II : apply the second diff + application.consume(mutationDiff()); + CHECK (join (subject.keys()) == "α, β, γ"); // the attributes weren't altered + scope = subject.scope(); // but the scope was reordered + CHECK ( *scope == CHILD_T); // CHILD_T + CHECK (*++scope == CHILD_A); // CHILD_A + Rec nested = (++scope)->data.get(); // and our nested Record, which too has been altered: + CHECK (nested.get("γ").data.get() == 3.45); // it carries now an attribute "δ", which is again + CHECK (nested.get("δ") == MakeRec().appendChild(CHILD_A) // a nested Record with three children CHILD_A + .appendChild(CHILD_A) // + .appendChild(CHILD_A) // + .genNode("δ")); // + auto subScope = nested.scope(); // and within the nested sub-scope we find + CHECK ( *subScope == CHILD_A); // CHILD_A + CHECK (*++subScope == MakeRec().type("Y") // a yet-again nested sub-Record of type "Y" + .set("β", 2L ) // with just an attribute "β" == 2L + .genNode(CHILD_NODE.idi.getSym())); // (and an empty child scope) + CHECK (*++subScope == CHILD_T); // followed by another copy of CHILD_T + CHECK (isnil (++subScope)); // + CHECK (isnil (++scope)); // and nothing beyond that. } }; diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index 1e4cae219..56a1bbdce 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -1072,10 +1072,10 @@ - + - + @@ -1711,9 +1711,9 @@ - - - + + + @@ -1846,8 +1846,8 @@ - - + + @@ -1882,7 +1882,7 @@ - + @@ -1892,8 +1892,8 @@ Problem: Rekursion

- - + + @@ -1920,8 +1920,7 @@ ist der gesammte Diff für den eingeschachtelten Kontext konsumiert

- - + @@ -1956,8 +1955,7 @@ und legt einen neuen Mutator an für den nested scope

- - + @@ -1984,8 +1982,7 @@ interner Stack

- - + @@ -2004,12 +2001,37 @@ also den TreeMutator, vermutlich das andere Modell (rekursiv konsumieren) verwenden.

- - + + + + + + + + + + + + +

+ Problem sind mal wieder die automatisch generierten IDs. +

+

+ Die sind natürlich anders, wenn wir die ganze Testsuite ausführen... +

+ + + + + + + + + @@ -2585,5 +2607,27 @@ + + + + + + + + + + +

+ sporadischer Fehlschlag am 1.11.2015 +

+

+ wir kann das passieren? +

+ + + + + +