From 3bb9101b07970bb65c30e566b713cbf2d1ce5635 Mon Sep 17 00:00:00 2001 From: Jocelyn Fiat Date: Tue, 12 Jan 2016 16:09:29 +0100 Subject: [PATCH] Improved error library by refactorying the sync as two way propagation. Now one can setup error handler propagation in one way, or two way (sync). The "reset" applies in both way, even if this is a one way propagation to fit current existing usage. Added optional id for the error handlers. Feature renaming according to design changes. Added related autotest cases. --- library/utility/general/error/package.iron | 14 +- .../utility/general/error/src/error_custom.e | 6 +- .../utility/general/error/src/error_handler.e | 358 ++++++++++++++---- .../utility/general/error/tests/test_error.e | 88 ++++- .../general/error/tests/tests-safe.ecf | 14 +- 5 files changed, 385 insertions(+), 95 deletions(-) diff --git a/library/utility/general/error/package.iron b/library/utility/general/error/package.iron index 3db2e673..c7f9322d 100644 --- a/library/utility/general/error/package.iron +++ b/library/utility/general/error/package.iron @@ -5,11 +5,13 @@ project error = "error.ecf" note --- title: --- description: --- tags: --- license: --- copyright: --- link[doc]: "Documentation" http:// + title: Error framework + description: "[ + Errors and associated handler, to manage errors and also provides a way to synchronize one or many error handlers. + This is convenient to propagate error from a layer to another without adding unwanted dependencies. + tags: error + license: Eiffel Forum License v2 + copyright: Jocelyn Fiat, Eiffel Software and others. + link[license]: http://www.eiffel.com/licensing/forum.txt end diff --git a/library/utility/general/error/src/error_custom.e b/library/utility/general/error/src/error_custom.e index 8c1330a2..f67fb2e9 100644 --- a/library/utility/general/error/src/error_custom.e +++ b/library/utility/general/error/src/error_custom.e @@ -16,13 +16,13 @@ create feature {NONE} -- Initialization - make (a_code: INTEGER; a_name: like name; a_message: detachable like message) + make (a_code: INTEGER; a_name: like name; a_message: detachable READABLE_STRING_GENERAL) -- Initialize `Current'. do code := a_code name := a_name if a_message /= Void then - message := a_message + message := a_message.as_string_32 else message := {STRING_32} "Error: " + a_name + " (code=" + a_code.out + ")" end @@ -45,7 +45,7 @@ feature -- Visitor end note - copyright: "2011-2012, Eiffel Software and others" + copyright: "2011-2016, Jocelyn Fiat, Eiffel Software and others" license: "Eiffel Forum License v2 (see http://www.eiffel.com/licensing/forum.txt)" source: "[ Eiffel Software diff --git a/library/utility/general/error/src/error_handler.e b/library/utility/general/error/src/error_handler.e index 50ae8f21..9cd9e1de 100644 --- a/library/utility/general/error/src/error_handler.e +++ b/library/utility/general/error/src/error_handler.e @@ -1,5 +1,8 @@ note - description : "Objects that handle error..." + description : "[ + Error handler or receiver. + + ]" legal: "See notice at end of class." status: "See notice at end of class." date: "$Date: 2015-10-10 00:55:41 +0200 (sam., 10 oct. 2015) $" @@ -14,7 +17,8 @@ inherit DEBUG_OUTPUT create - make + make, + make_with_id feature {NONE} -- Initialization @@ -25,8 +29,22 @@ feature {NONE} -- Initialization create error_added_actions end + make_with_id (a_id: READABLE_STRING_8) + -- Build `Current' with optional id `a_id'. + do + make + if a_id = Void then + id := Void + else + create id.make_from_string (a_id) + end + end + feature -- Access + id: detachable IMMUTABLE_STRING_8 + -- Optional identifier for Current handler. + primary_error_code: INTEGER -- Code of first error in `errors' require @@ -49,6 +67,19 @@ feature -- Status Result := errors.count end + is_synchronizing_with (other: ERROR_HANDLER): BOOLEAN + -- Is Current synchronizing with `other'? + -- i.e 2 way propagation. + do + Result := is_propagating_to (other) and other.is_propagating_to (Current) + end + + is_propagating_to (other: ERROR_HANDLER): BOOLEAN + -- Is Current propagating error to `other'? + do + Result := attached propagations as lst and then lst.has (other) + end + feature {ERROR_HANDLER, ERROR_VISITOR} -- Restricted access errors: LIST [ERROR] @@ -64,6 +95,25 @@ feature -- Status report else Result := "no error" end + if attached id as l_id then + Result.prepend ("[" + l_id + "] ") + end + if attached propagations as lst then + check not_empty: not lst.is_empty end + Result.append_character ('(') + Result.append (" -> ") + Result.append_integer (lst.count) + Result.append_character (':') + across + lst as ic + loop + if attached ic.item.id as l_id then + Result.append_character (' ') + Result.append (l_id) + end + end + Result.append_character (')') + end end feature -- Events @@ -77,134 +127,263 @@ feature -- Synchronization -- Add synchronization between `h' and `Current' --| the same handler can be added more than once --| it will be synchronized only once - local - lst: like synchronized_handlers do - lst := synchronized_handlers - if lst = Void then - create {ARRAYED_LIST [ERROR_HANDLER]} lst.make (0) - lst.compare_references - synchronized_handlers := lst - end - if lst.has (h) then - check attached h.synchronized_handlers as h_lst and then h_lst.has (Current) end - else - lst.extend (h) - h.add_synchronization (Current) - end + add_propagation (h) + h.add_propagation (Current) end remove_synchronization (h: ERROR_HANDLER) -- Remove synchronization between `h' and `Current' do - if attached synchronized_handlers as lst and then not lst.is_empty then - synchronized_handlers := Void + remove_propagation (h) + h.remove_propagation (Current) + end + +feature -- One way synchronization: propagation + + add_propagation (h: ERROR_HANDLER) + -- Add propagation from `Current' to `h'. + --| the same handler can be added more than once + --| it will be synchronized only once + local + lst: like propagations + do + h.register_propagator (Current) + lst := propagations + if lst = Void then + create {ARRAYED_LIST [ERROR_HANDLER]} lst.make (0) + lst.compare_references + propagations := lst + end + if not lst.has (h) then + lst.extend (h) + end + end + + remove_propagation (h: ERROR_HANDLER) + -- Remove propagation from `Current' to `h'. + do + if attached propagations as lst and then not lst.is_empty then lst.prune_all (h) - - h.remove_synchronization (Current) - - synchronized_handlers := lst if lst.is_empty then - synchronized_handlers := Void + propagations := Void end end + h.unregister_propagator (Current) end feature {ERROR_HANDLER} -- Synchronization implementation - synchronized_handlers: detachable LIST [ERROR_HANDLER] - -- Synchronized handlers - - synchronize_error_from (e: ERROR; h_lst: LIST [ERROR_HANDLER]) - -- Called by error_handler during synchronization process - -- if `synchronized_handlers' is Void, this means Current is synchronizing - -- this is to prevent infinite cycle iteration - require - not h_lst.has (Current) + is_associated_with (h: ERROR_HANDLER): BOOLEAN do - h_lst.extend (Current) - - if attached synchronized_handlers as lst then - synchronized_handlers := Void - add_error (e) - across - lst as c - loop - if not h_lst.has (c.item) then - c.item.synchronize_error_from (e, h_lst) - end - end - synchronized_handlers := lst - else - -- In synchronization + if attached propagators as lst then + Result := lst.has (h) end end - synchronize_reset_from (h_lst: LIST [ERROR_HANDLER]) - -- Called by error_handler during synchronization process - -- if `synchronized_handlers' is Void, this means Current is synchronizing - -- this is to prevent infinite cycle iteration + register_propagator (h: ERROR_HANDLER) + local + lst: like propagators + do + lst := propagators + if lst = Void then + create {ARRAYED_LIST [ERROR_HANDLER]} lst.make (1) + propagators := lst + end + if not lst.has (h) then + lst.extend (h) + end + end + + unregister_propagator (h: ERROR_HANDLER) + local + lst: like propagators + do + lst := propagators + if lst /= Void then + lst.prune_all (h) + if lst.is_empty then + propagators := Void + end + end + end + + propagators: detachable LIST [ERROR_HANDLER] + -- Handlers propagating to Current. + -- Needed for `reset'. + + propagations: detachable LIST [ERROR_HANDLER] + -- Handlers receiving the propagation. + + propagate_error_addition (e: ERROR; h_lst: LIST [ERROR_HANDLER]) + -- Called by error_handler during synchronization process. + -- To prevent infinite cycle, if Currently synchronizing, the `propagations' is Void. require not h_lst.has (Current) + local + lst: like propagations do h_lst.extend (Current) - if attached synchronized_handlers as lst then - synchronized_handlers := Void - reset + lst := propagations + propagations := Void + add_error (e) + + if lst /= Void then across lst as c loop if not h_lst.has (c.item) then - c.item.synchronize_reset_from (h_lst) + c.item.propagate_error_addition (e, h_lst) end end - synchronized_handlers := lst + propagations := lst else - -- In synchronization + --| propagating + end + end + + propagate_errors_removal (errs: ITERABLE [ERROR]; h_lst: LIST [ERROR_HANDLER]) + -- Called by error_handler during synchronization process. + -- To prevent infinite cycle, if Currently synchronizing, the `propagations' is Void. + require + not h_lst.has (Current) + local + lst: like propagations + do + h_lst.extend (Current) + lst := propagations + propagations := Void + across + errs as ic + loop + -- Question: should we use remove_error (ic.item) ? + errors.prune_all (ic.item) + end + + if lst /= Void then + across + lst as c + loop + if not h_lst.has (c.item) then + c.item.propagate_errors_removal (errs, h_lst) + end + end + propagations := lst + else + --| propagating + end + end + + backpropagate_reset (h_lst: LIST [ERROR_HANDLER]) + -- Called by error_handler during propagation process. + -- To prevent infinite cycle, if Currently synchronizing, the `propagations' is Void. + require + not h_lst.has (Current) + local + lst: like propagations + do + h_lst.extend (Current) + lst := propagations + propagations := Void + reset + if lst /= Void then + across + lst as c + loop + if not h_lst.has (c.item) then + -- Reset c.item, even if this is not a two way synchronization! + c.item.backpropagate_reset (h_lst) + end + end + propagations := lst + else + --| propagating end end feature {NONE} -- Event: implementation on_error_added (e: ERROR) - -- Error `e' was just added + -- Error `e' was just added. local - sync_list: LINKED_LIST [ERROR_HANDLER] + sync_list: ARRAYED_LIST [ERROR_HANDLER] do error_added_actions.call ([e]) - if attached synchronized_handlers as lst then - synchronized_handlers := Void - create sync_list.make + if attached propagations as lst then + propagations := Void + create sync_list.make (1 + lst.count) sync_list.extend (Current) across lst as c loop if not sync_list.has (c.item) then - c.item.synchronize_error_from (e, sync_list) + c.item.propagate_error_addition (e, sync_list) end end - synchronized_handlers := lst + propagations := lst + end + end + + on_errors_removed (errs: ITERABLE [ERROR]) + -- Errors `errs' were just removed. + local + sync_list: ARRAYED_LIST [ERROR_HANDLER] + lst: like propagations + do + lst := propagations + if lst /= Void then + propagations := Void + create sync_list.make (1 + lst.count) + sync_list.extend (Current) + across + lst as c + loop + if not sync_list.has (c.item) then + c.item.propagate_errors_removal (errs, sync_list) + end + end + propagations := lst end end on_reset -- `reset' was just called local - sync_list: LINKED_LIST [ERROR_HANDLER] + sync_list: detachable ARRAYED_LIST [ERROR_HANDLER] + lst: detachable LIST [ERROR_HANDLER] do - if attached synchronized_handlers as lst then - synchronized_handlers := Void - create sync_list.make + lst := propagators + if lst /= Void then + create sync_list.make (1 + lst.count) sync_list.extend (Current) + + propagators := Void across lst as c loop if not sync_list.has (c.item) then - c.item.synchronize_reset_from (sync_list) + c.item.backpropagate_reset (sync_list) end end - synchronized_handlers := lst + propagators := lst end + +-- lst := propagations +-- propagations := Void +-- if lst /= Void then +-- if sync_list = Void then +-- create sync_list.make (1 + lst.count) +-- sync_list.extend (Current) +-- end +-- across +-- lst as c +-- loop +-- if not sync_list.has (c.item) then +-- c.item.synchronize_reset_from (sync_list) +-- end +-- end +-- propagations := lst +-- end end feature -- Basic operation @@ -216,7 +395,17 @@ feature -- Basic operation on_error_added (a_error) end - add_error_details, add_custom_error (a_code: INTEGER; a_name: STRING; a_message: detachable READABLE_STRING_32) + remove_error (a_error: ERROR) + -- Remove `a_error' from the stack of error. + -- And also propagate error removal. + do + if propagations /= Void then + on_errors_removed (<>) + end + errors.prune_all (a_error) + end + + add_error_details, add_custom_error (a_code: INTEGER; a_name: STRING; a_message: detachable READABLE_STRING_GENERAL) -- Add custom error to the stack of error local e: ERROR_CUSTOM @@ -285,11 +474,22 @@ feature -- Element changes end reset - -- Reset error handler + -- Reset Current error handler. + -- And also reset recursively error handlers propagating to Current (i.e the propagators). + do + errors.wipe_out + on_reset + ensure + has_no_error: not has_error + count = 0 + end + + remove_all_errors + -- Remove all errors. do if errors.count > 0 then + on_errors_removed (errors) errors.wipe_out - on_reset end ensure has_no_error: not has_error @@ -297,22 +497,26 @@ feature -- Element changes end destroy - -- Destroy Current, and remove any synchronization + -- Destroy Current, and remove any propagations (in the two directions). do error_added_actions.wipe_out - if attached synchronized_handlers as lst then + if attached propagations as lst then + propagations := Void across lst as c loop c.item.remove_synchronization (Current) end end - synchronized_handlers := Void reset end +invariant + propagations_not_empty: attached propagations as lst implies not lst.is_empty + propagators_not_empty: attached propagators as lst implies not lst.is_empty + note - copyright: "2011-2014, Jocelyn Fiat, Eiffel Software and others" + copyright: "2011-2016, Jocelyn Fiat, Eiffel Software and others" license: "Eiffel Forum License v2 (see http://www.eiffel.com/licensing/forum.txt)" source: "[ Eiffel Software diff --git a/library/utility/general/error/tests/test_error.e b/library/utility/general/error/tests/test_error.e index af7fdec8..8f671695 100644 --- a/library/utility/general/error/tests/test_error.e +++ b/library/utility/general/error/tests/test_error.e @@ -38,6 +38,30 @@ feature -- Test routines end + test_propagation_2 + note + testing: "error" + local + h1, h2: ERROR_HANDLER +-- cl: CELL [INTEGER] + do + create h1.make_with_id ("h1") + create h2.make_with_id ("h2") + + h1.add_propagation (h2) + + h1.add_custom_error (123, "abc", "abc error occurred") + h1.add_custom_error (456, "def", "def error occurred") + + assert ("has 2 errors", h1.count = 2 and h2.count = h1.count) + + h2.add_custom_error (789, "ghi", "error occurred only for h2") + assert ("h1 has 2 errors, h2 has 3 errors", h1.count = 2 and h2.count = h1.count + 1) + + h1.remove_all_errors + assert ("remove all error from h1, and one remaining on h2", h1.count = 0 and h2.count = 1) + end + test_sync_2 note testing: "error" @@ -50,7 +74,7 @@ feature -- Test routines h1.add_synchronization (h2) h1.add_custom_error (123, "abc", "abc error occurred") - h1.add_custom_error (456, "abc", "abc error occurred") + h1.add_custom_error (456, "def", "def error occurred") assert ("has 2 errors", h1.count = 2 and h2.count = h1.count) @@ -108,6 +132,62 @@ feature -- Test routines end + test_tree_propag + --| Testing tree propagation + --| + --| |-----> batch .......... + --| | : + --| db ---> core ---> app ----> ui : + --| : : : : : + --| :.......:.........:........:...:.....> logger + --| + note + testing: "error" + local + hdb, hcore, happ, hui, hlog, hbat: ERROR_HANDLER + do + create hdb.make_with_id ("db") + create hcore.make_with_id ("core") + create happ.make_with_id ("app") + create hui.make_with_id ("ui") + create hlog.make_with_id ("logger") + create hbat.make_with_id ("batch") + + hdb.add_propagation (hcore) + hcore.add_propagation (happ) + hcore.add_propagation (hbat) + happ.add_propagation (hui) + + hdb.add_propagation (hlog) + hcore.add_propagation (hlog) + happ.add_propagation (hlog) + hui.add_propagation (hlog) + hbat.add_propagation (hlog) + + hdb.add_custom_error (1, "db", "database connection error") + hdb.add_custom_error (2, "db", "database query error") + assert ("2 errors", hdb.count = 2 and hcore.count = 2 and happ.count = 2 and hui.count = 2 and hlog.count = 2) + hcore.add_custom_error (11, "core", "core error") + assert ("3 errors", hdb.count = 2 and hcore.count = 3 and happ.count = 3 and hui.count = 3 and hlog.count = 3) + happ.add_custom_error (101, "app", "app issue") + assert ("2 errors", hdb.count = 2 and hcore.count = 3 and happ.count = 4 and hui.count = 4 and hlog.count = 4) + hui.add_custom_error (1001, "ui", "user interface trouble") + assert ("2 errors", hdb.count = 2 and hcore.count = 3 and happ.count = 4 and hui.count = 5 and hlog.count = 5) + + assert ("hbat", hbat.count = 3 and hlog.count = 5) + hbat.add_custom_error (10001, "bat", "Batch issue") + assert ("2 errors", hdb.count = 2 and hcore.count = 3 and happ.count = 4 and hui.count = 5 and hbat.count = 4 and hlog.count = 6) + hbat.remove_all_errors + assert ("2 errors", hdb.count = 2 and hcore.count = 3 and happ.count = 4 and hui.count = 5 and hbat.count = 0 and hlog.count = 2) + + hui.remove_all_errors + assert ("2 errors", hdb.count = 2 and hcore.count = 3 and happ.count = 4 and hui.count = 0 and hlog.count = 0) + + hdb.reset + assert ("no more errors", hdb.count = 0 and hcore.count = 3 and happ.count = 4 and hui.count = 0 and hlog.count = 0) + + end + test_remove_sync note testing: "error" @@ -145,9 +225,9 @@ feature -- Test routines h1, h2, h3: ERROR_HANDLER -- cl: CELL [INTEGER] do - create h1.make - create h2.make - create h3.make + create h1.make_with_id ("h1") + create h2.make_with_id ("h2") + create h3.make_with_id ("h3") h1.add_synchronization (h2) h2.add_synchronization (h3) diff --git a/library/utility/general/error/tests/tests-safe.ecf b/library/utility/general/error/tests/tests-safe.ecf index 26d20921..1ba33c90 100644 --- a/library/utility/general/error/tests/tests-safe.ecf +++ b/library/utility/general/error/tests/tests-safe.ecf @@ -1,17 +1,21 @@ - + /.git$ - /EIFGENs$ /.svn$ + /EIFGENs$ -