[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
This commit is contained in:
Robert von Burg 2016-11-03 13:35:29 +01:00
parent da41e2aeb9
commit e7cf9bad6d
11 changed files with 32 additions and 12 deletions

View File

@ -31,6 +31,7 @@ import li.strolch.utils.dbc.DBC;
public class AddActivityCommand extends Command { public class AddActivityCommand extends Command {
private Activity activity; private Activity activity;
private boolean added;
/** /**
* @param tx * @param tx
@ -64,11 +65,12 @@ public class AddActivityCommand extends Command {
} }
activityMap.add(tx(), this.activity); activityMap.add(tx(), this.activity);
this.added = true;
} }
@Override @Override
public void undo() { public void undo() {
if (this.activity != null && tx().isRollingBack()) { if (this.added && this.activity != null && tx().isRollingBack()) {
ActivityMap activityMap = tx().getActivityMap(); ActivityMap activityMap = tx().getActivityMap();
if (activityMap.hasElement(tx(), this.activity.getType(), this.activity.getId())) if (activityMap.hasElement(tx(), this.activity.getType(), this.activity.getId()))
activityMap.remove(tx(), this.activity); activityMap.remove(tx(), this.activity);

View File

@ -32,6 +32,7 @@ import li.strolch.utils.dbc.DBC;
public class AddOrderCollectionCommand extends Command { public class AddOrderCollectionCommand extends Command {
private List<Order> orders; private List<Order> orders;
private boolean added;
/** /**
* @param tx * @param tx
@ -69,11 +70,12 @@ public class AddOrderCollectionCommand extends Command {
} }
orderMap.addAll(tx(), this.orders); orderMap.addAll(tx(), this.orders);
this.added = true;
} }
@Override @Override
public void undo() { 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(); OrderMap orderMap = tx().getOrderMap();
for (Order order : this.orders) { for (Order order : this.orders) {
if (orderMap.hasElement(tx(), order.getType(), order.getId())) { if (orderMap.hasElement(tx(), order.getType(), order.getId())) {

View File

@ -31,6 +31,7 @@ import li.strolch.utils.dbc.DBC;
public class AddOrderCommand extends Command { public class AddOrderCommand extends Command {
private Order order; private Order order;
private boolean added;
/** /**
* @param tx * @param tx
@ -64,11 +65,12 @@ public class AddOrderCommand extends Command {
} }
orderMap.add(tx(), this.order); orderMap.add(tx(), this.order);
this.added = true;
} }
@Override @Override
public void undo() { public void undo() {
if (this.order != null && tx().isRollingBack()) { if (this.added && this.order != null && tx().isRollingBack()) {
OrderMap orderMap = tx().getOrderMap(); OrderMap orderMap = tx().getOrderMap();
if (orderMap.hasElement(tx(), this.order.getType(), this.order.getId())) if (orderMap.hasElement(tx(), this.order.getType(), this.order.getId()))
orderMap.remove(tx(), this.order); orderMap.remove(tx(), this.order);

View File

@ -32,6 +32,7 @@ import li.strolch.utils.dbc.DBC;
public class AddResourceCollectionCommand extends Command { public class AddResourceCollectionCommand extends Command {
private List<Resource> resources; private List<Resource> resources;
private boolean added;
/** /**
* @param tx * @param tx
@ -69,11 +70,12 @@ public class AddResourceCollectionCommand extends Command {
} }
resourceMap.addAll(tx(), this.resources); resourceMap.addAll(tx(), this.resources);
this.added = true;
} }
@Override @Override
public void undo() { 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(); ResourceMap resourceMap = tx().getResourceMap();
for (Resource resource : this.resources) { for (Resource resource : this.resources) {
if (resourceMap.hasElement(tx(), resource.getType(), resource.getId())) { if (resourceMap.hasElement(tx(), resource.getType(), resource.getId())) {

View File

@ -31,6 +31,7 @@ import li.strolch.utils.dbc.DBC;
public class AddResourceCommand extends Command { public class AddResourceCommand extends Command {
private Resource resource; private Resource resource;
private boolean added;
/** /**
* @param tx * @param tx
@ -64,11 +65,12 @@ public class AddResourceCommand extends Command {
} }
resourceMap.add(tx(), this.resource); resourceMap.add(tx(), this.resource);
this.added = true;
} }
@Override @Override
public void undo() { public void undo() {
if (this.resource != null && tx().isRollingBack()) { if (this.added && this.resource != null && tx().isRollingBack()) {
ResourceMap resourceMap = tx().getResourceMap(); ResourceMap resourceMap = tx().getResourceMap();
if (resourceMap.hasElement(tx(), this.resource.getType(), this.resource.getId())) if (resourceMap.hasElement(tx(), this.resource.getType(), this.resource.getId()))
resourceMap.remove(tx(), this.resource); resourceMap.remove(tx(), this.resource);

View File

@ -32,6 +32,7 @@ import li.strolch.utils.dbc.DBC;
public class RemoveActivityCollectionCommand extends Command { public class RemoveActivityCollectionCommand extends Command {
private List<Activity> activities; private List<Activity> activities;
private boolean removed;
/** /**
* @param tx * @param tx
@ -70,11 +71,12 @@ public class RemoveActivityCollectionCommand extends Command {
} }
activityMap.removeAll(tx(), this.activities); activityMap.removeAll(tx(), this.activities);
this.removed = true;
} }
@Override @Override
public void undo() { 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(); ActivityMap activityMap = tx().getActivityMap();
for (Activity activity : this.activities) { for (Activity activity : this.activities) {
if (!activityMap.hasElement(tx(), activity.getType(), activity.getId())) { if (!activityMap.hasElement(tx(), activity.getType(), activity.getId())) {

View File

@ -32,6 +32,7 @@ import li.strolch.utils.dbc.DBC;
public class RemoveOrderCollectionCommand extends Command { public class RemoveOrderCollectionCommand extends Command {
private List<Order> orders; private List<Order> orders;
private boolean removed;
/** /**
* @param tx * @param tx
@ -70,11 +71,12 @@ public class RemoveOrderCollectionCommand extends Command {
} }
orderMap.removeAll(tx(), this.orders); orderMap.removeAll(tx(), this.orders);
this.removed = true;
} }
@Override @Override
public void undo() { 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(); OrderMap orderMap = tx().getOrderMap();
for (Order order : this.orders) { for (Order order : this.orders) {
if (!orderMap.hasElement(tx(), order.getType(), order.getId())) { if (!orderMap.hasElement(tx(), order.getType(), order.getId())) {

View File

@ -32,6 +32,7 @@ import li.strolch.utils.dbc.DBC;
public class RemoveResourceCollectionCommand extends Command { public class RemoveResourceCollectionCommand extends Command {
private List<Resource> resources; private List<Resource> resources;
private boolean removed;
/** /**
* @param tx * @param tx
@ -70,11 +71,12 @@ public class RemoveResourceCollectionCommand extends Command {
} }
resourceMap.removeAll(tx(), this.resources); resourceMap.removeAll(tx(), this.resources);
this.removed = true;
} }
@Override @Override
public void undo() { 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(); ResourceMap resourceMap = tx().getResourceMap();
for (Resource resource : this.resources) { for (Resource resource : this.resources) {
if (!resourceMap.hasElement(tx(), resource.getType(), resource.getId())) { if (!resourceMap.hasElement(tx(), resource.getType(), resource.getId())) {

View File

@ -35,6 +35,7 @@ public class AddParameterCommand extends Command {
private ParameterizedElement element; private ParameterizedElement element;
private Parameter<?> parameter; private Parameter<?> parameter;
private boolean added;
/** /**
* @param container * @param container
@ -80,11 +81,12 @@ public class AddParameterCommand extends Command {
this.element.addParameter(this.parameter); this.element.addParameter(this.parameter);
new UpdateElementVisitor(tx()).update(rootElement); new UpdateElementVisitor(tx()).update(rootElement);
this.added = true;
} }
@Override @Override
public void undo() { public void undo() {
if (this.parameter != null) { if (this.added && this.parameter != null) {
if (this.element.hasParameter(this.parameter.getId())) { if (this.element.hasParameter(this.parameter.getId())) {
this.element.removeParameter(this.parameter.getId()); this.element.removeParameter(this.parameter.getId());
new UndoUpdateElementVisitor(tx()).undo(this.element.getRootElement()); new UndoUpdateElementVisitor(tx()).undo(this.element.getRootElement());

View File

@ -37,6 +37,7 @@ public class RemoveParameterCommand extends Command {
private String parameterId; private String parameterId;
private Parameter<?> removedParameter; private Parameter<?> removedParameter;
private boolean removed;
/** /**
* @param container * @param container
@ -82,11 +83,12 @@ public class RemoveParameterCommand extends Command {
this.removedParameter = this.element.removeParameter(this.parameterId); this.removedParameter = this.element.removeParameter(this.parameterId);
new UpdateElementVisitor(tx()).update(rootElement); new UpdateElementVisitor(tx()).update(rootElement);
this.removed = true;
} }
@Override @Override
public void undo() { public void undo() {
if (this.removedParameter != null) { if (this.removed && this.removedParameter != null) {
this.element.addParameter(this.removedParameter); this.element.addParameter(this.removedParameter);
new UndoUpdateElementVisitor(tx()).undo(this.removedParameter.getRootElement()); new UndoUpdateElementVisitor(tx()).undo(this.removedParameter.getRootElement());
} }

View File

@ -165,7 +165,7 @@ public class SetParameterCommand extends Command {
@Override @Override
public void undo() { public void undo() {
if (this.parameter != null) { if (this.updated && this.parameter != null) {
if (this.oldName != null) { if (this.oldName != null) {
this.parameter.setName(this.oldName); this.parameter.setName(this.oldName);
} }
@ -187,7 +187,7 @@ public class SetParameterCommand extends Command {
visitor.setValue(this.parameter, this.oldValueAsString); visitor.setValue(this.parameter, this.oldValueAsString);
} }
if (hasChanges() && this.updated) { if (hasChanges()) {
StrolchRootElement rootElement = this.parameter.getRootElement(); StrolchRootElement rootElement = this.parameter.getRootElement();
new UndoUpdateElementVisitor(tx()).undo(rootElement); new UndoUpdateElementVisitor(tx()).undo(rootElement);
} }