From cef7917d8e256fc2d863a4f8e569ca776161b918 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sun, 15 Dec 2019 21:40:09 +0100 Subject: [PATCH] Diff-Listener: finished and unit test pass (closes: #1206) --- src/lib/diff/tree-diff.hpp | 15 +- .../diff/tree-mutator-listener-binding.hpp | 6 +- src/lib/diff/tree-mutator.hpp | 4 +- tests/15library.tests | 12 +- .../diff/diff-tree-mutation-listener-test.cpp | 67 ++++-- wiki/thinkPad.ichthyo.mm | 200 +++++++++++------- 6 files changed, 201 insertions(+), 103 deletions(-) diff --git a/src/lib/diff/tree-diff.hpp b/src/lib/diff/tree-diff.hpp index 95d933692..d8ad601b0 100644 --- a/src/lib/diff/tree-diff.hpp +++ b/src/lib/diff/tree-diff.hpp @@ -32,14 +32,15 @@ ** - 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. + ** attributes first, followed by the children. The end of the attribute scope is marked + ** by the the first emerging 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). + ** optionally a name (and a named element counts as 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. + ** a \em nested record ("nested child object"). In case of value elements, + ** however, the element itself is identified with this value payload. + ** - the typing of the elements is outside the scope of the diff language; it is assumed + ** that the receiver of the diff knows what types to expect and how to deal with them. ** - there is a notion of changing or mutating the data content, while retaining ** the identity of the element. Of course this requires the data content to be ** assignable, which makes content mutation an optional feature. @@ -53,7 +54,7 @@ ** a diff sequence to it. We provide a standard implementation of the ** DiffApplicator + DiffApplicationStrategy, based on a _custimisable intermediary,_ ** the TreeMutator. This allows to apply a given tree to any suitably compatible - ** target data structure; especially there is a preconfigured setup for our + ** target data structure; notably there is a preconfigured setup for our ** _"generic tree representation"_, diff::Record. ** ** @see diff-language.cpp diff --git a/src/lib/diff/tree-mutator-listener-binding.hpp b/src/lib/diff/tree-mutator-listener-binding.hpp index 2fdf6563f..071ab3e88 100644 --- a/src/lib/diff/tree-mutator-listener-binding.hpp +++ b/src/lib/diff/tree-mutator-listener-binding.hpp @@ -32,7 +32,7 @@ ** adaptation is done by combining various building blocks. This header defines ** a special decorator to be layered on top of such a TreeMutator binding; it will ** not interfere with the received diff, but detect relevant changes and invoke the - ** bound functor after the triggering diff has been applied completely. + ** functor after the triggering diff has been applied completely to the bound scope. ** ** @note the header tree-mutator-listener-binding.hpp was split off for sake of readability ** and is included automatically from bottom of tree-mutator.hpp -- no need to @@ -67,6 +67,8 @@ namespace diff{ * API operation intercepted here. * @note TreeMutator is a disposable one-way object; * the triggering mechanism directly relies on that. + * The listener is invoked, whenever a scope is complete, + * including processing of any nested scopes. */ template class Detector4StructuralChanges @@ -109,7 +111,7 @@ namespace diff{ template template inline auto - Builder::onStructuralChange (LIS changeListener) + Builder::onSeqChange (LIS changeListener) { ASSERT_VALID_SIGNATURE (LIS, void(void)) diff --git a/src/lib/diff/tree-mutator.hpp b/src/lib/diff/tree-mutator.hpp index ff85cf34b..7b67dda8f 100644 --- a/src/lib/diff/tree-mutator.hpp +++ b/src/lib/diff/tree-mutator.hpp @@ -459,10 +459,10 @@ namespace diff{ * @remark in theory, one could also drop contents indirectly, by sending only * part of the necessary PICK verbs (or using AFTER(...)). However, * such a diff is formally not prohibited, and will indeed be detected as - * error in many but not all cases, in ChildCollectionMutator::completeScope() + * error when leaving a scope, in ChildCollectionMutator::completeScope() */ template - auto onStructuralChange (LIS changeListener); + auto onSeqChange (LIS changeListener); }; }//(END) Mutator-Builder... diff --git a/tests/15library.tests b/tests/15library.tests index f351002a1..2d761c233 100644 --- a/tests/15library.tests +++ b/tests/15library.tests @@ -63,7 +63,12 @@ return: 0 END -TEST "Diff: reshape a tree data structure through diff" DiffTreeApplication_test < #include #include @@ -53,13 +53,26 @@ namespace test{ VAL_C{"c"}, VAL_D{"d"}, - C_TO_B{"c", "B"}; + VAL_C_UPPER{"C"}, + VAL_D_UPPER{"D"}; string contents (vector const& strings) { return util::join (strings); } + + string + lowerCase (string src) + { + return boost::algorithm::to_lower_copy(src); //WARNING: only works for ASCII + } + + bool + caseInsensitiveEqual (string a, string b) + { + return lowerCase (a) == lowerCase (b); + } }//(End)Test fixture @@ -71,13 +84,13 @@ namespace test{ /****************************************************************************//** - * @test When creating a TreeMutator binding, a listener (lambda) can be atteched, + * @test When creating a TreeMutator binding, a listener (lambda) can be attached, * to be invoked on structural changes... * - inserting, removing and reordering of children counts as "structural" change * - whereas assignment of a new value or nested mutation does not trigger * @note This test binds the test class itself for diff mutation, applying changes * onto a vector with test data. The binding itself is somewhat unusual, - * insofar it allows to re-assign elements within the vector, can be + * insofar it allows to re-assign elements within the vector, which can be * identified and picked by equality match. In actual code, you would not * do that, since typically you'd distinguish between attributes, which * are marked by an identifier and can be reassigned, and children, which @@ -85,6 +98,10 @@ namespace test{ * as such does not enforce such conventions; if you want to find a * sub-element, you need to provide a _matcher_ to identify it, * given a suitable "spec" in the relevant diff verbs. + * @remark Now the special rigging for this test is that we match case-insensitively, + * which allows to assign a different value, while this value still counts as + * "equal", as far as matching is concerned. We do all this trickery in order + * to apply a diff, which _changes values_ while not _changing the structure_. * @see DiffTreeApplicationSimple_test introductory example demonstration * @see DiffTreeApplication_test extended demonstration of possible diff operations * @see DiffComplexApplication_test handling arbitrary data structures @@ -99,13 +116,29 @@ namespace test{ std::vector subject_; int structChanges_ = 0; + /** rig the test class itself to receive a diff mutation. + * - bind the #subject_ data collection to be changed by diff + * - attach a listener, to be invoked on _structural changes + */ void buildMutator (TreeMutator::Handle buff) override { buff.create ( TreeMutator::build() - .attach (collection (subject_)) - .onStructuralChange ([&](){ ++structChanges_; }) + .attach (collection (subject_) + .matchElement ([](GenNode const& spec, string const& elm) -> bool + { // »Matcher« : what target string "matches" a diff spec? + return caseInsensitiveEqual(elm, spec.data.get()); + }) + .assignElement ([](string& target, GenNode const& spec) -> bool + { // »Setter« : how to assign the value from the spec to the target + target = spec.data.get(); + return true; + })) + .onSeqChange ([&]() + { + ++structChanges_; // Note: this lambda is the key point for this test + }) ); } @@ -127,18 +160,26 @@ namespace test{ CHECK (1 == structChanges_); applicator.consume (MutationMessage{{after(Ref::END) - , set (C_TO_B) + , set (VAL_C_UPPER) // Note: the current element is tried first, which happens to match + , set (VAL_D_UPPER) // ...while in this case, a linear search finds the "d" }}); - CHECK ("a, B, d, c" == contents(subject_)); - CHECK (1 == structChanges_); + CHECK ("a, c, D, C" == contents(subject_)); + CHECK (1 == structChanges_); // Note: the listener has not fired, since this counts as value change. applicator.consume (MutationMessage{{pick(VAL_A) , ins (VAL_B) - , find(VAL_C) - , after(Ref::END) + , find(VAL_D) + , pick(VAL_C) + , skip(VAL_D) + , del (VAL_C) }}); - CHECK ("a, b, c, B, d" == contents(subject_)); - CHECK (2 == structChanges_); + CHECK ("a, b, D, c" == contents(subject_)); + CHECK (2 == structChanges_); // Note: this obviously is a structure change, so the listener fired. + + applicator.consume (MutationMessage{{after(Ref::END) + }}); + CHECK ("a, b, D, c" == contents(subject_)); + CHECK (2 == structChanges_); // Note: contents confirmed as-is, listener not invoked. } }; diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index d6ab7ba1a..2470be5f4 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -19721,8 +19721,8 @@ - - + + @@ -21790,9 +21790,9 @@ - + - + @@ -21820,8 +21820,8 @@ - - + + @@ -35926,8 +35926,8 @@ - - + + @@ -35961,21 +35961,57 @@ - - - - - + + + + + + +

+ brauche Detektor für structural change +

+ + + + + + + + + + + + + + + + +

+ ...man könnte auch auf die Idee kommen, es nur in das Collection-Binding einzuhängen. +

+

+ Das wäre aber zu kurz gedacht; auch wenn im Moment dieses die einzige Implementierung ist, die den Listener tatsächlich triggern kann, verbietet uns niemand in der Zukunft, noch eine anderes TreeMutator-Binding zu erfinden. Hinzu kommt, daß ein Listener zusätzliche Storage und zusätzlichen Aufwand bedeutet, und deshalb besser als eigener »onion layer« implementiert wird +

+ + + + + + + + + + - - + + @@ -35989,8 +36025,7 @@ und zwar etwas wirklich Einfaches

- - + @@ -36002,8 +36037,7 @@ denn "elementar" bedeutet in diesem Fall, es wird ziemlich technisch und komplex

- - + @@ -36033,8 +36067,7 @@ Jetzt gibt es also echte ContainerTraits, und damit ist der Aufbau zukunftsfest.

- - + @@ -36048,36 +36081,13 @@ - - + + - - - - - - - - -

- ...man könnte auch auf die Idee kommen, es nur in das Collection-Binding einzuhängen. -

-

- Das wäre aber zu kurz gedacht; auch wenn im Moment dieses die einzige Implementierung ist, die den Listener tatsächlich triggern kann, verbietet uns niemand in der Zukunft, noch eine anderes TreeMutator-Binding zu erfinden. Hinzu kommt, daß ein Listener zusätzliche Storage und zusätzlichen Aufwand bedeutet, und deshalb besser als eigener »onion layer« implementiert wird -

- - - - - - - - - - - + + @@ -36112,8 +36122,7 @@ Dann wir der Rest mit dem Mutator zusammen weggeworfen.

- - + @@ -36125,8 +36134,7 @@ um das zu unterbinden, müßten alle Binding-Layer kollaborieren

- - + @@ -36142,8 +36150,7 @@ dieses aber hätte auch die notwendigen Informationen, um diese Situation zu erkennen

- - + @@ -36160,8 +36167,7 @@ d.h. es prüft, ob keine Elemente im Arbeitspuffer übrig sind

- - + @@ -36213,8 +36219,7 @@ Und mühsam für den Leser ist er so oder so... dann also lieber alles ausschreiben

- - + @@ -36241,8 +36246,7 @@ und damit wiederholt sich das Ketten-Argument bis zur Basisklasse runter!!!

- - + @@ -36268,8 +36272,7 @@ selbst wenn er über x Basisklassen von MoveOnly erbt

- - + @@ -36283,8 +36286,8 @@ - - + + @@ -36301,23 +36304,63 @@ - - + + - - - - - - - + + + + + + + + + + + + + + + + + + +

+ matchElement([](GenNode const& spec, ELM const& elm) +

+

+                          { +

+

+                            return spec.matches(elm); +

+

+                          }) +

+ + + + + + + + + + - - + + + + + + + + + @@ -36336,9 +36379,10 @@ - - - + + + +