From e7cf9bad6dca7ead7ccb025a00cc077651c67496 Mon Sep 17 00:00:00 2001 From: Robert von Burg Date: Thu, 3 Nov 2016 13:35:29 +0100 Subject: [PATCH] [Fix] Fixed undo logic for general commands Should a command be undone, then some commands performed an undo, although they didn't perform their work, this led to an inconsistent data model. I.e. AddResourceCommand would fail because the resource already existed and an undo would lead to the existing object being removed --- .../src/main/java/li/strolch/command/AddActivityCommand.java | 4 +++- .../java/li/strolch/command/AddOrderCollectionCommand.java | 4 +++- .../src/main/java/li/strolch/command/AddOrderCommand.java | 4 +++- .../java/li/strolch/command/AddResourceCollectionCommand.java | 4 +++- .../src/main/java/li/strolch/command/AddResourceCommand.java | 4 +++- .../li/strolch/command/RemoveActivityCollectionCommand.java | 4 +++- .../java/li/strolch/command/RemoveOrderCollectionCommand.java | 4 +++- .../li/strolch/command/RemoveResourceCollectionCommand.java | 4 +++- .../li/strolch/command/parameter/AddParameterCommand.java | 4 +++- .../li/strolch/command/parameter/RemoveParameterCommand.java | 4 +++- .../li/strolch/command/parameter/SetParameterCommand.java | 4 ++-- 11 files changed, 32 insertions(+), 12 deletions(-) diff --git a/li.strolch.service/src/main/java/li/strolch/command/AddActivityCommand.java b/li.strolch.service/src/main/java/li/strolch/command/AddActivityCommand.java index 7dd5ccc36..f5744434d 100644 --- a/li.strolch.service/src/main/java/li/strolch/command/AddActivityCommand.java +++ b/li.strolch.service/src/main/java/li/strolch/command/AddActivityCommand.java @@ -31,6 +31,7 @@ import li.strolch.utils.dbc.DBC; public class AddActivityCommand extends Command { private Activity activity; + private boolean added; /** * @param tx @@ -64,11 +65,12 @@ public class AddActivityCommand extends Command { } activityMap.add(tx(), this.activity); + this.added = true; } @Override public void undo() { - if (this.activity != null && tx().isRollingBack()) { + if (this.added && this.activity != null && tx().isRollingBack()) { ActivityMap activityMap = tx().getActivityMap(); if (activityMap.hasElement(tx(), this.activity.getType(), this.activity.getId())) activityMap.remove(tx(), this.activity); diff --git a/li.strolch.service/src/main/java/li/strolch/command/AddOrderCollectionCommand.java b/li.strolch.service/src/main/java/li/strolch/command/AddOrderCollectionCommand.java index 9c70b8a9d..1d00f4509 100644 --- a/li.strolch.service/src/main/java/li/strolch/command/AddOrderCollectionCommand.java +++ b/li.strolch.service/src/main/java/li/strolch/command/AddOrderCollectionCommand.java @@ -32,6 +32,7 @@ import li.strolch.utils.dbc.DBC; public class AddOrderCollectionCommand extends Command { private List orders; + private boolean added; /** * @param tx @@ -69,11 +70,12 @@ public class AddOrderCollectionCommand extends Command { } orderMap.addAll(tx(), this.orders); + this.added = true; } @Override public void undo() { - if (this.orders != null && !this.orders.isEmpty() && tx().isRollingBack()) { + if (this.added && this.orders != null && !this.orders.isEmpty() && tx().isRollingBack()) { OrderMap orderMap = tx().getOrderMap(); for (Order order : this.orders) { if (orderMap.hasElement(tx(), order.getType(), order.getId())) { diff --git a/li.strolch.service/src/main/java/li/strolch/command/AddOrderCommand.java b/li.strolch.service/src/main/java/li/strolch/command/AddOrderCommand.java index 7b8e30de0..a35afea79 100644 --- a/li.strolch.service/src/main/java/li/strolch/command/AddOrderCommand.java +++ b/li.strolch.service/src/main/java/li/strolch/command/AddOrderCommand.java @@ -31,6 +31,7 @@ import li.strolch.utils.dbc.DBC; public class AddOrderCommand extends Command { private Order order; + private boolean added; /** * @param tx @@ -64,11 +65,12 @@ public class AddOrderCommand extends Command { } orderMap.add(tx(), this.order); + this.added = true; } @Override public void undo() { - if (this.order != null && tx().isRollingBack()) { + if (this.added && this.order != null && tx().isRollingBack()) { OrderMap orderMap = tx().getOrderMap(); if (orderMap.hasElement(tx(), this.order.getType(), this.order.getId())) orderMap.remove(tx(), this.order); diff --git a/li.strolch.service/src/main/java/li/strolch/command/AddResourceCollectionCommand.java b/li.strolch.service/src/main/java/li/strolch/command/AddResourceCollectionCommand.java index 72a5d18fb..4906652fd 100644 --- a/li.strolch.service/src/main/java/li/strolch/command/AddResourceCollectionCommand.java +++ b/li.strolch.service/src/main/java/li/strolch/command/AddResourceCollectionCommand.java @@ -32,6 +32,7 @@ import li.strolch.utils.dbc.DBC; public class AddResourceCollectionCommand extends Command { private List resources; + private boolean added; /** * @param tx @@ -69,11 +70,12 @@ public class AddResourceCollectionCommand extends Command { } resourceMap.addAll(tx(), this.resources); + this.added = true; } @Override public void undo() { - if (this.resources != null && !this.resources.isEmpty() && tx().isRollingBack()) { + if (this.added && this.resources != null && !this.resources.isEmpty() && tx().isRollingBack()) { ResourceMap resourceMap = tx().getResourceMap(); for (Resource resource : this.resources) { if (resourceMap.hasElement(tx(), resource.getType(), resource.getId())) { diff --git a/li.strolch.service/src/main/java/li/strolch/command/AddResourceCommand.java b/li.strolch.service/src/main/java/li/strolch/command/AddResourceCommand.java index 7b9615a48..822d0bac8 100644 --- a/li.strolch.service/src/main/java/li/strolch/command/AddResourceCommand.java +++ b/li.strolch.service/src/main/java/li/strolch/command/AddResourceCommand.java @@ -31,6 +31,7 @@ import li.strolch.utils.dbc.DBC; public class AddResourceCommand extends Command { private Resource resource; + private boolean added; /** * @param tx @@ -64,11 +65,12 @@ public class AddResourceCommand extends Command { } resourceMap.add(tx(), this.resource); + this.added = true; } @Override public void undo() { - if (this.resource != null && tx().isRollingBack()) { + if (this.added && this.resource != null && tx().isRollingBack()) { ResourceMap resourceMap = tx().getResourceMap(); if (resourceMap.hasElement(tx(), this.resource.getType(), this.resource.getId())) resourceMap.remove(tx(), this.resource); diff --git a/li.strolch.service/src/main/java/li/strolch/command/RemoveActivityCollectionCommand.java b/li.strolch.service/src/main/java/li/strolch/command/RemoveActivityCollectionCommand.java index b745ff2ea..7789d680b 100644 --- a/li.strolch.service/src/main/java/li/strolch/command/RemoveActivityCollectionCommand.java +++ b/li.strolch.service/src/main/java/li/strolch/command/RemoveActivityCollectionCommand.java @@ -32,6 +32,7 @@ import li.strolch.utils.dbc.DBC; public class RemoveActivityCollectionCommand extends Command { private List activities; + private boolean removed; /** * @param tx @@ -70,11 +71,12 @@ public class RemoveActivityCollectionCommand extends Command { } activityMap.removeAll(tx(), this.activities); + this.removed = true; } @Override public void undo() { - if (this.activities != null && !this.activities.isEmpty() && tx().isRollingBack()) { + if (this.removed && this.activities != null && !this.activities.isEmpty() && tx().isRollingBack()) { ActivityMap activityMap = tx().getActivityMap(); for (Activity activity : this.activities) { if (!activityMap.hasElement(tx(), activity.getType(), activity.getId())) { diff --git a/li.strolch.service/src/main/java/li/strolch/command/RemoveOrderCollectionCommand.java b/li.strolch.service/src/main/java/li/strolch/command/RemoveOrderCollectionCommand.java index 1fca5b8e9..9dcdca323 100644 --- a/li.strolch.service/src/main/java/li/strolch/command/RemoveOrderCollectionCommand.java +++ b/li.strolch.service/src/main/java/li/strolch/command/RemoveOrderCollectionCommand.java @@ -32,6 +32,7 @@ import li.strolch.utils.dbc.DBC; public class RemoveOrderCollectionCommand extends Command { private List orders; + private boolean removed; /** * @param tx @@ -70,11 +71,12 @@ public class RemoveOrderCollectionCommand extends Command { } orderMap.removeAll(tx(), this.orders); + this.removed = true; } @Override public void undo() { - if (this.orders != null && !this.orders.isEmpty() && tx().isRollingBack()) { + if (this.removed && this.orders != null && !this.orders.isEmpty() && tx().isRollingBack()) { OrderMap orderMap = tx().getOrderMap(); for (Order order : this.orders) { if (!orderMap.hasElement(tx(), order.getType(), order.getId())) { diff --git a/li.strolch.service/src/main/java/li/strolch/command/RemoveResourceCollectionCommand.java b/li.strolch.service/src/main/java/li/strolch/command/RemoveResourceCollectionCommand.java index 873cb5954..663892f02 100644 --- a/li.strolch.service/src/main/java/li/strolch/command/RemoveResourceCollectionCommand.java +++ b/li.strolch.service/src/main/java/li/strolch/command/RemoveResourceCollectionCommand.java @@ -32,6 +32,7 @@ import li.strolch.utils.dbc.DBC; public class RemoveResourceCollectionCommand extends Command { private List resources; + private boolean removed; /** * @param tx @@ -70,11 +71,12 @@ public class RemoveResourceCollectionCommand extends Command { } resourceMap.removeAll(tx(), this.resources); + this.removed = true; } @Override public void undo() { - if (this.resources != null && !this.resources.isEmpty() && tx().isRollingBack()) { + if (this.removed && this.resources != null && !this.resources.isEmpty() && tx().isRollingBack()) { ResourceMap resourceMap = tx().getResourceMap(); for (Resource resource : this.resources) { if (!resourceMap.hasElement(tx(), resource.getType(), resource.getId())) { diff --git a/li.strolch.service/src/main/java/li/strolch/command/parameter/AddParameterCommand.java b/li.strolch.service/src/main/java/li/strolch/command/parameter/AddParameterCommand.java index 1e0495a4c..6bb8cc8ed 100644 --- a/li.strolch.service/src/main/java/li/strolch/command/parameter/AddParameterCommand.java +++ b/li.strolch.service/src/main/java/li/strolch/command/parameter/AddParameterCommand.java @@ -35,6 +35,7 @@ public class AddParameterCommand extends Command { private ParameterizedElement element; private Parameter parameter; + private boolean added; /** * @param container @@ -80,11 +81,12 @@ public class AddParameterCommand extends Command { this.element.addParameter(this.parameter); new UpdateElementVisitor(tx()).update(rootElement); + this.added = true; } @Override public void undo() { - if (this.parameter != null) { + if (this.added && this.parameter != null) { if (this.element.hasParameter(this.parameter.getId())) { this.element.removeParameter(this.parameter.getId()); new UndoUpdateElementVisitor(tx()).undo(this.element.getRootElement()); diff --git a/li.strolch.service/src/main/java/li/strolch/command/parameter/RemoveParameterCommand.java b/li.strolch.service/src/main/java/li/strolch/command/parameter/RemoveParameterCommand.java index f3858267c..8937433cd 100644 --- a/li.strolch.service/src/main/java/li/strolch/command/parameter/RemoveParameterCommand.java +++ b/li.strolch.service/src/main/java/li/strolch/command/parameter/RemoveParameterCommand.java @@ -37,6 +37,7 @@ public class RemoveParameterCommand extends Command { private String parameterId; private Parameter removedParameter; + private boolean removed; /** * @param container @@ -82,11 +83,12 @@ public class RemoveParameterCommand extends Command { this.removedParameter = this.element.removeParameter(this.parameterId); new UpdateElementVisitor(tx()).update(rootElement); + this.removed = true; } @Override public void undo() { - if (this.removedParameter != null) { + if (this.removed && this.removedParameter != null) { this.element.addParameter(this.removedParameter); new UndoUpdateElementVisitor(tx()).undo(this.removedParameter.getRootElement()); } diff --git a/li.strolch.service/src/main/java/li/strolch/command/parameter/SetParameterCommand.java b/li.strolch.service/src/main/java/li/strolch/command/parameter/SetParameterCommand.java index 421ee9ec9..89686ba1e 100644 --- a/li.strolch.service/src/main/java/li/strolch/command/parameter/SetParameterCommand.java +++ b/li.strolch.service/src/main/java/li/strolch/command/parameter/SetParameterCommand.java @@ -165,7 +165,7 @@ public class SetParameterCommand extends Command { @Override public void undo() { - if (this.parameter != null) { + if (this.updated && this.parameter != null) { if (this.oldName != null) { this.parameter.setName(this.oldName); } @@ -187,7 +187,7 @@ public class SetParameterCommand extends Command { visitor.setValue(this.parameter, this.oldValueAsString); } - if (hasChanges() && this.updated) { + if (hasChanges()) { StrolchRootElement rootElement = this.parameter.getRootElement(); new UndoUpdateElementVisitor(tx()).undo(rootElement); }