From bbf021f73b920a52fc4c18aa658b189a02f9b09e Mon Sep 17 00:00:00 2001 From: Robert von Burg Date: Fri, 22 Jun 2018 11:38:24 +0200 Subject: [PATCH] [Fix] Fixed bug where changed element is not returned in streams --- .../persistence/api/AbstractTransaction.java | 18 +- .../li/strolch/search/StrolchSearchTest.java | 2 +- .../src/main/java/li/strolch/model/Order.java | 10 +- .../main/java/li/strolch/model/Resource.java | 10 +- .../li/strolch/model/activity/Activity.java | 12 +- .../java/li/strolch/service/TxModifyTest.java | 160 ++++++++++++++++++ 6 files changed, 198 insertions(+), 14 deletions(-) create mode 100644 li.strolch.service/src/test/java/li/strolch/service/TxModifyTest.java diff --git a/li.strolch.agent/src/main/java/li/strolch/persistence/api/AbstractTransaction.java b/li.strolch.agent/src/main/java/li/strolch/persistence/api/AbstractTransaction.java index 8311ea5b2..457487aca 100644 --- a/li.strolch.agent/src/main/java/li/strolch/persistence/api/AbstractTransaction.java +++ b/li.strolch.agent/src/main/java/li/strolch/persistence/api/AbstractTransaction.java @@ -514,17 +514,29 @@ public abstract class AbstractTransaction implements StrolchTransaction { @Override public Stream streamResources(String... types) { - return getResourceMap().stream(this, types); + return getResourceMap().stream(this, types).map(e -> { + // perhaps the element was changed before, so we check if it is in the filter first + Resource element = getElementFromFilter(Tags.RESOURCE, e.getLocator()); + return element == null ? e : element; + }); } @Override public Stream streamOrders(String... types) { - return getOrderMap().stream(this, types); + return getOrderMap().stream(this, types).map(e -> { + // perhaps the element was changed before, so we check if it is in the filter first + Order element = getElementFromFilter(Tags.ORDER, e.getLocator()); + return element == null ? e : element; + }); } @Override public Stream streamActivities(String... types) { - return getActivityMap().stream(this, types); + return getActivityMap().stream(this, types).map(e -> { + // perhaps the element was changed before, so we check if it is in the filter first + Activity element = getElementFromFilter(Tags.ACTIVITY, e.getLocator()); + return element == null ? e : element; + }); } @Override diff --git a/li.strolch.agent/src/test/java/li/strolch/search/StrolchSearchTest.java b/li.strolch.agent/src/test/java/li/strolch/search/StrolchSearchTest.java index accf5b4f0..e82288e55 100644 --- a/li.strolch.agent/src/test/java/li/strolch/search/StrolchSearchTest.java +++ b/li.strolch.agent/src/test/java/li/strolch/search/StrolchSearchTest.java @@ -276,7 +276,7 @@ public class StrolchSearchTest { Map states = new ActivitySearch() - .types().where(state().isEqualTo(State.PLANNING).and(name(isEqualTo("Activity"))).asActivityExp()) + .types().where(state().isEqualTo(State.PLANNING).and(name(isEqualTo("Activity"))).asActivity()) .search(tx).toMap(Activity::getId, Activity::getState); diff --git a/li.strolch.model/src/main/java/li/strolch/model/Order.java b/li.strolch.model/src/main/java/li/strolch/model/Order.java index 494219579..6e92f9274 100644 --- a/li.strolch.model/src/main/java/li/strolch/model/Order.java +++ b/li.strolch.model/src/main/java/li/strolch/model/Order.java @@ -39,6 +39,7 @@ public class Order extends AbstractStrolchRootElement implements StrolchRootElem private static final long serialVersionUID = 0L; + protected Locator locator; protected Version version; protected Date date; protected State state; @@ -204,9 +205,12 @@ public class Order extends AbstractStrolchRootElement implements StrolchRootElem @Override public Locator getLocator() { - LocatorBuilder lb = new LocatorBuilder(); - fillLocator(lb); - return lb.build(); + if (this.locator == null) { + LocatorBuilder lb = new LocatorBuilder(); + fillLocator(lb); + this.locator = lb.build(); + } + return this.locator; } @Override diff --git a/li.strolch.model/src/main/java/li/strolch/model/Resource.java b/li.strolch.model/src/main/java/li/strolch/model/Resource.java index 582219d8b..28aa0851d 100644 --- a/li.strolch.model/src/main/java/li/strolch/model/Resource.java +++ b/li.strolch.model/src/main/java/li/strolch/model/Resource.java @@ -41,6 +41,7 @@ public class Resource extends AbstractStrolchRootElement implements StrolchRootE private static final long serialVersionUID = 0L; + protected Locator locator; protected Version version; protected Map>> timedStateMap; protected PolicyDefs policyDefs; @@ -240,9 +241,12 @@ public class Resource extends AbstractStrolchRootElement implements StrolchRootE @Override public Locator getLocator() { - LocatorBuilder lb = new LocatorBuilder(); - fillLocator(lb); - return lb.build(); + if (this.locator == null) { + LocatorBuilder lb = new LocatorBuilder(); + fillLocator(lb); + this.locator = lb.build(); + } + return this.locator; } @Override diff --git a/li.strolch.model/src/main/java/li/strolch/model/activity/Activity.java b/li.strolch.model/src/main/java/li/strolch/model/activity/Activity.java index 720412104..9b49168ce 100644 --- a/li.strolch.model/src/main/java/li/strolch/model/activity/Activity.java +++ b/li.strolch.model/src/main/java/li/strolch/model/activity/Activity.java @@ -42,7 +42,8 @@ public class Activity extends AbstractStrolchRootElement private static final long serialVersionUID = 1L; - private Version version; + protected Locator locator; + protected Version version; protected Activity parent; protected TimeOrdering timeOrdering; @@ -453,9 +454,12 @@ public class Activity extends AbstractStrolchRootElement @Override public Locator getLocator() { - LocatorBuilder lb = new LocatorBuilder(); - fillLocator(lb); - return lb.build(); + if (this.locator == null) { + LocatorBuilder lb = new LocatorBuilder(); + fillLocator(lb); + this.locator = lb.build(); + } + return this.locator; } @Override diff --git a/li.strolch.service/src/test/java/li/strolch/service/TxModifyTest.java b/li.strolch.service/src/test/java/li/strolch/service/TxModifyTest.java new file mode 100644 index 000000000..5c11169be --- /dev/null +++ b/li.strolch.service/src/test/java/li/strolch/service/TxModifyTest.java @@ -0,0 +1,160 @@ +/* + * Copyright 2015 Robert von Burg + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package li.strolch.service; + +import static org.junit.Assert.*; + +import java.util.List; + +import li.strolch.model.ModelGenerator; +import li.strolch.model.Resource; +import li.strolch.persistence.api.AddResourceCommand; +import li.strolch.persistence.api.RemoveResourceCommand; +import li.strolch.persistence.api.StrolchTransaction; +import li.strolch.persistence.api.UpdateResourceCommand; +import li.strolch.search.ResourceSearch; +import li.strolch.service.api.AbstractService; +import li.strolch.service.api.Service; +import li.strolch.service.api.ServiceArgument; +import li.strolch.service.api.ServiceResult; +import li.strolch.service.test.AbstractRealmServiceTest; +import li.strolch.utils.dbc.DBC; +import org.junit.Assert; +import org.junit.Test; + +public class TxModifyTest extends AbstractRealmServiceTest { + + private Class> svcClass; + + @Test + public void shouldFindModifiedElementInSearch() { + this.svcClass = ModifyAndSearchService.class; + runServiceInAllRealmTypes(); + } + + @Test + public void shouldRollbackSuccessfully() { + this.svcClass = RollbackService.class; + runServiceInAllRealmTypes(); + } + + @Override + protected Class> getSvcClass() { + return this.svcClass; + } + + @Override + protected ServiceArgument getArgInstance() { + return new ServiceArgument(); + } + + public static class ModifyAndSearchService extends AbstractService { + private static final long serialVersionUID = 1L; + + @Override + protected ServiceResult getResultInstance() { + return new ServiceResult(); + } + + @Override + public ServiceArgument getArgumentInstance() { + return new ServiceArgument(); + } + + @Override + protected ServiceResult internalDoService(ServiceArgument arg) throws Exception { + + String type = "Car"; + String id = "car001"; + Resource resource = ModelGenerator.createResource(id, id, type); + try (StrolchTransaction tx = openTx(arg.realm)) { + tx.add(resource); + tx.commitOnClose(); + } + + try (StrolchTransaction tx = openTx(arg.realm)) { + + // get the element + Resource car001 = tx.getResourceBy(type, id, true); + + // modify + car001.setName("Changed!"); + tx.update(car001); + + // search for all cars + List cars = new ResourceSearch().types(type).search(tx).cloneIfReadOnly().toList(); + assertEquals(1, cars.size()); + assertSame("We didn't get the same car!", car001, cars.get(0)); + + // also validate get returns the same instance + Resource carByGet = tx.getResourceBy(type, id, true); + assertSame("We didn't get the same car!", car001, carByGet); + + // also validate find returns the same instance + Resource carByFind = tx.findElement(car001.getLocator()); + assertSame("We didn't get the same car!", car001, carByFind); + + tx.commitOnClose(); + } + + return ServiceResult.success(); + } + } + + public static class RollbackService extends AbstractService { + private static final long serialVersionUID = 1L; + + @Override + protected ServiceResult getResultInstance() { + return new ServiceResult(); + } + + @Override + public ServiceArgument getArgumentInstance() { + return new ServiceArgument(); + } + + @Override + protected ServiceResult internalDoService(ServiceArgument arg) throws Exception { + + String type = "Car"; + String id = "car002"; + Resource resource = ModelGenerator.createResource(id, id, type); + try (StrolchTransaction tx = openTx(arg.realm)) { + tx.add(resource); + tx.commitOnClose(); + } + + try (StrolchTransaction tx = openTx(arg.realm)) { + Resource car001 = tx.getResourceBy(type, id, true); + + // modify + car001.setName("Changed!"); + tx.update(car001); + + tx.rollbackOnClose(); + } + + // now make sure the element was not changed + try (StrolchTransaction tx = openTx(arg.realm)) { + Resource car001 = tx.getResourceBy(type, id, true); + assertEquals("Expected name not to be changed in rollback of TX!", id, car001.getName()); + } + + return ServiceResult.success(); + } + } +}