From 7a56059036ebc38497fe23c701d1554adb6c26b8 Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Mon, 13 Nov 2017 15:55:24 -0500 Subject: [PATCH] Fix misuse of Drafted interface in tests. WIP fixing use of immutable collections in UPDATE/draft logic. --- .../net/helenus/core/AbstractEntityDraft.java | 7 +--- .../net/helenus/core/AbstractUnitOfWork.java | 17 ++++---- .../operation/AbstractOptionalOperation.java | 40 +++++++++---------- .../operation/AbstractStreamOperation.java | 38 ++++++++---------- .../core/operation/InsertOperation.java | 3 +- .../core/operation/UpdateOperation.java | 32 +++++++++++---- .../net/helenus/mapping/HelenusEntity.java | 2 + .../helenus/mapping/HelenusMappingEntity.java | 16 ++++++++ .../java/net/helenus/mapping/MappingUtil.java | 23 ----------- .../mapping/value/ValueProviderMap.java | 3 +- .../integration/core/draft/Inventory.java | 4 +- .../test/integration/core/draft/Supply.java | 5 +-- .../core/unitofwork/UnitOfWorkTest.java | 3 +- .../helenus/test/unit/core/dsl/Account.java | 4 +- 14 files changed, 96 insertions(+), 101 deletions(-) diff --git a/src/main/java/net/helenus/core/AbstractEntityDraft.java b/src/main/java/net/helenus/core/AbstractEntityDraft.java index 40075ec..8f2a449 100644 --- a/src/main/java/net/helenus/core/AbstractEntityDraft.java +++ b/src/main/java/net/helenus/core/AbstractEntityDraft.java @@ -51,12 +51,7 @@ public abstract class AbstractEntityDraft implements Drafted { } else { // Collections fetched from the entityMap if (value instanceof Collection) { - try { - value = MappingUtil.clone(value); - } catch (CloneNotSupportedException e) { - // TODO(gburd): deep?shallow? copy of List, Map, Set to a mutable collection. - value = (T) SerializationUtils.clone((Serializable) value); - } + value = (T) SerializationUtils.clone((Serializable) value); } } } diff --git a/src/main/java/net/helenus/core/AbstractUnitOfWork.java b/src/main/java/net/helenus/core/AbstractUnitOfWork.java index 3025a73..b9f5b27 100644 --- a/src/main/java/net/helenus/core/AbstractUnitOfWork.java +++ b/src/main/java/net/helenus/core/AbstractUnitOfWork.java @@ -22,6 +22,7 @@ import com.google.common.base.Stopwatch; import com.google.common.collect.HashBasedTable; import com.google.common.collect.Table; import com.google.common.collect.TreeTraverser; +import java.io.Serializable; import java.util.*; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -30,9 +31,9 @@ import net.helenus.core.cache.CacheUtil; import net.helenus.core.cache.Facet; import net.helenus.core.operation.AbstractOperation; import net.helenus.core.operation.BatchOperation; -import net.helenus.core.reflect.Drafted; import net.helenus.mapping.MappingUtil; import net.helenus.support.Either; +import org.apache.commons.lang3.SerializationUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -218,15 +219,11 @@ public abstract class AbstractUnitOfWork result = checkParentCache(facets); if (result.isPresent()) { Object r = result.get(); - try { - Class iface = MappingUtil.getMappingInterface(r); - if (Drafted.class.isAssignableFrom(iface)) { - cacheUpdate(r, facets); - } else { - cacheUpdate(MappingUtil.clone(r), facets); - } - } catch (CloneNotSupportedException e) { - result = Optional.empty(); + Class iface = MappingUtil.getMappingInterface(r); + if (Helenus.entity(iface).isDraftable()) { + cacheUpdate(r, facets); + } else { + cacheUpdate(SerializationUtils.clone((Serializable) r), facets); } } return result; diff --git a/src/main/java/net/helenus/core/operation/AbstractOptionalOperation.java b/src/main/java/net/helenus/core/operation/AbstractOptionalOperation.java index 911383f..afe577d 100644 --- a/src/main/java/net/helenus/core/operation/AbstractOptionalOperation.java +++ b/src/main/java/net/helenus/core/operation/AbstractOptionalOperation.java @@ -24,18 +24,20 @@ import com.google.common.base.Function; import com.google.common.base.Stopwatch; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import java.io.Serializable; import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.TimeoutException; import net.helenus.core.AbstractSessionOperations; +import net.helenus.core.Helenus; import net.helenus.core.UnitOfWork; import net.helenus.core.cache.CacheUtil; import net.helenus.core.cache.Facet; -import net.helenus.core.reflect.Drafted; import net.helenus.mapping.MappingUtil; import net.helenus.support.Fun; +import org.apache.commons.lang3.SerializationUtils; public abstract class AbstractOptionalOperation> extends AbstractStatementOperation { @@ -153,26 +155,22 @@ public abstract class AbstractOptionalOperation iface = MappingUtil.getMappingInterface(cachedResult); - try { - if (Drafted.class.isAssignableFrom(iface)) { - result = Optional.of(cachedResult); - } else { - result = Optional.of(MappingUtil.clone(cachedResult)); - } - sessionCacheHits.mark(); - cacheHits.mark(); - uow.recordCacheAndDatabaseOperationCount(1, 0); - } catch (CloneNotSupportedException e) { - result = Optional.empty(); - sessionCacheMiss.mark(); - cacheMiss.mark(); - uow.recordCacheAndDatabaseOperationCount(-1, 0); - } finally { - if (result.isPresent()) { - updateCache = true; - } else { - updateCache = false; - } + if (Helenus.entity(iface).isDraftable()) { + result = Optional.of(cachedResult); + } else { + result = + Optional.of( + (E) + SerializationUtils.clone( + (Serializable) cachedResult)); + } + sessionCacheHits.mark(); + cacheHits.mark(); + uow.recordCacheAndDatabaseOperationCount(1, 0); + if (result.isPresent()) { + updateCache = true; + } else { + updateCache = false; } } else { updateCache = false; diff --git a/src/main/java/net/helenus/core/operation/AbstractStreamOperation.java b/src/main/java/net/helenus/core/operation/AbstractStreamOperation.java index f0dac94..07b5649 100644 --- a/src/main/java/net/helenus/core/operation/AbstractStreamOperation.java +++ b/src/main/java/net/helenus/core/operation/AbstractStreamOperation.java @@ -24,6 +24,7 @@ import com.google.common.base.Function; import com.google.common.base.Stopwatch; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import java.io.Serializable; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -31,12 +32,13 @@ import java.util.concurrent.CompletionException; import java.util.concurrent.TimeoutException; import java.util.stream.Stream; import net.helenus.core.AbstractSessionOperations; +import net.helenus.core.Helenus; import net.helenus.core.UnitOfWork; import net.helenus.core.cache.CacheUtil; import net.helenus.core.cache.Facet; -import net.helenus.core.reflect.Drafted; import net.helenus.mapping.MappingUtil; import net.helenus.support.Fun; +import org.apache.commons.lang3.SerializationUtils; public abstract class AbstractStreamOperation> extends AbstractStatementOperation { @@ -160,26 +162,20 @@ public abstract class AbstractStreamOperation iface = MappingUtil.getMappingInterface(cachedResult); E result = null; - try { - if (Drafted.class.isAssignableFrom(iface)) { - result = cachedResult; - } else { - result = MappingUtil.clone(cachedResult); - } - resultStream = Stream.of(result); - sessionCacheHits.mark(); - cacheHits.mark(); - uow.recordCacheAndDatabaseOperationCount(1, 0); - } catch (CloneNotSupportedException e) { - resultStream = null; - sessionCacheMiss.mark(); - uow.recordCacheAndDatabaseOperationCount(-1, 0); - } finally { - if (result != null) { - updateCache = true; - } else { - updateCache = false; - } + if (Helenus.entity(iface).isDraftable()) { + result = cachedResult; + } else { + result = + (E) SerializationUtils.clone((Serializable) cachedResult); + } + resultStream = Stream.of(result); + sessionCacheHits.mark(); + cacheHits.mark(); + uow.recordCacheAndDatabaseOperationCount(1, 0); + if (result != null) { + updateCache = true; + } else { + updateCache = false; } } else { updateCache = false; diff --git a/src/main/java/net/helenus/core/operation/InsertOperation.java b/src/main/java/net/helenus/core/operation/InsertOperation.java index 5676c01..3547f82 100644 --- a/src/main/java/net/helenus/core/operation/InsertOperation.java +++ b/src/main/java/net/helenus/core/operation/InsertOperation.java @@ -31,7 +31,6 @@ import net.helenus.core.cache.CacheUtil; import net.helenus.core.cache.Facet; import net.helenus.core.cache.UnboundFacet; import net.helenus.core.reflect.DefaultPrimitiveTypes; -import net.helenus.core.reflect.Drafted; import net.helenus.core.reflect.HelenusPropertyNode; import net.helenus.core.reflect.MapExportable; import net.helenus.mapping.HelenusEntity; @@ -211,7 +210,7 @@ public final class InsertOperation extends AbstractOperation iface) { if (values.size() > 0) { - boolean immutable = iface.isAssignableFrom(Drafted.class); + boolean immutable = entity.isDraftable(); Collection properties = entity.getOrderedProperties(); Map backingMap = new HashMap(properties.size()); diff --git a/src/main/java/net/helenus/core/operation/UpdateOperation.java b/src/main/java/net/helenus/core/operation/UpdateOperation.java index 6288baa..fa946a2 100644 --- a/src/main/java/net/helenus/core/operation/UpdateOperation.java +++ b/src/main/java/net/helenus/core/operation/UpdateOperation.java @@ -203,7 +203,9 @@ public final class UpdateOperation extends AbstractFilterOperation) draftMap.get(key); + list = + (List) new ArrayList((List) draftMap.get(key)); // copy immutable -> mutable list + draft.put(key, list); list.add(0, value); facet = new BoundFacet(prop, list); } else { @@ -235,7 +237,9 @@ public final class UpdateOperation extends AbstractFilterOperation 0) { String key = p.getProperty().getPropertyName(); - list = (List) draftMap.get(key); + list = + (List) new ArrayList((List) draftMap.get(key)); // copy immutable -> mutable list + draft.put(key, list); list.addAll(0, value); facet = new BoundFacet(prop, list); } else { @@ -266,7 +270,10 @@ public final class UpdateOperation extends AbstractFilterOperation) BeanColumnValueProvider.INSTANCE.getColumnValue(pojo, -1, prop, false); } else { String key = prop.getPropertyName(); - list = (List) draftMap.get(key); + list = + (List) + new ArrayList((List) draftMap.get(key)); // copy immutable -> mutable list + draft.put(key, list); } if (idx < 0) { list.add(0, value); @@ -305,7 +312,9 @@ public final class UpdateOperation extends AbstractFilterOperation) draftMap.get(key); + list = + (List) new ArrayList((List) draftMap.get(key)); // copy immutable -> mutable list + draft.put(key, list); list.add(value); facet = new BoundFacet(prop, list); } else { @@ -336,7 +345,9 @@ public final class UpdateOperation extends AbstractFilterOperation 0) { String key = prop.getPropertyName(); - list = (List) draftMap.get(key); + list = + (List) new ArrayList((List) draftMap.get(key)); // copy immutable -> mutable list + draft.put(key, list); list.addAll(value); facet = new BoundFacet(prop, list); } else { @@ -367,7 +378,9 @@ public final class UpdateOperation extends AbstractFilterOperation) draftMap.get(key); + list = + (List) new ArrayList((List) draftMap.get(key)); // copy immutable -> mutable list + draft.put(key, list); list.remove(value); facet = new BoundFacet(prop, list); } else { @@ -398,7 +411,9 @@ public final class UpdateOperation extends AbstractFilterOperation) draftMap.get(key); + list = + (List) new ArrayList((List) draftMap.get(key)); // copy immutable -> mutable list + draft.put(key, list); list.removeAll(value); facet = new BoundFacet(prop, list); } else { @@ -467,7 +482,8 @@ public final class UpdateOperation extends AbstractFilterOperation) draftMap.get(key); + set = (Set) new HashSet((Set) draftMap.get(key)); + draft.put(key, set); set.add(value); facet = new BoundFacet(prop, set); } else { diff --git a/src/main/java/net/helenus/mapping/HelenusEntity.java b/src/main/java/net/helenus/mapping/HelenusEntity.java index b19fee6..9ec0744 100644 --- a/src/main/java/net/helenus/mapping/HelenusEntity.java +++ b/src/main/java/net/helenus/mapping/HelenusEntity.java @@ -34,4 +34,6 @@ public interface HelenusEntity { HelenusProperty getProperty(String name); List getFacets(); + + boolean isDraftable(); } diff --git a/src/main/java/net/helenus/mapping/HelenusMappingEntity.java b/src/main/java/net/helenus/mapping/HelenusMappingEntity.java index 989a010..965178d 100644 --- a/src/main/java/net/helenus/mapping/HelenusMappingEntity.java +++ b/src/main/java/net/helenus/mapping/HelenusMappingEntity.java @@ -39,6 +39,7 @@ public final class HelenusMappingEntity implements HelenusEntity { private final HelenusEntityType type; private final IdentityName name; private final boolean cacheable; + private final boolean draftable; private final ImmutableMap methods; private final ImmutableMap props; private final ImmutableList orderedProps; @@ -112,6 +113,16 @@ public final class HelenusMappingEntity implements HelenusEntity { // Caching cacheable = (null != iface.getDeclaredAnnotation(Cacheable.class)); + // Draft + Class draft; + try { + draft = Class.forName(iface.getName() + "$Draft"); + } catch (Exception ignored) { + draft = null; + } + draftable = (draft != null); + + // Materialized view List primaryKeyProperties = new ArrayList<>(); ImmutableList.Builder facetsBuilder = ImmutableList.builder(); if (iface.getDeclaredAnnotation(MaterializedView.class) == null) { @@ -212,6 +223,11 @@ public final class HelenusMappingEntity implements HelenusEntity { return cacheable; } + @Override + public boolean isDraftable() { + return draftable; + } + @Override public Class getMappingInterface() { return iface; diff --git a/src/main/java/net/helenus/mapping/MappingUtil.java b/src/main/java/net/helenus/mapping/MappingUtil.java index 08b4bac..bb68587 100644 --- a/src/main/java/net/helenus/mapping/MappingUtil.java +++ b/src/main/java/net/helenus/mapping/MappingUtil.java @@ -16,7 +16,6 @@ package net.helenus.mapping; import java.lang.annotation.Annotation; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; @@ -305,28 +304,6 @@ public final class MappingUtil { } } - // https://stackoverflow.com/a/4882306/366692 - public static T clone(T object) throws CloneNotSupportedException { - Object clone = null; - - // Use reflection, because there is no other way - try { - Method method = object.getClass().getMethod("clone"); - clone = method.invoke(object); - } catch (InvocationTargetException e) { - rethrow(e.getCause()); - } catch (Exception cause) { - rethrow(cause); - } - if (object.getClass().isInstance(clone)) { - @SuppressWarnings("unchecked") // clone class <= object class <= T - T t = (T) clone; - return t; - } else { - throw new ClassCastException(clone.getClass().getName()); - } - } - private static void rethrow(Throwable cause) throws CloneNotSupportedException { if (cause instanceof RuntimeException) { throw (RuntimeException) cause; diff --git a/src/main/java/net/helenus/mapping/value/ValueProviderMap.java b/src/main/java/net/helenus/mapping/value/ValueProviderMap.java index 43e6318..eb1388c 100644 --- a/src/main/java/net/helenus/mapping/value/ValueProviderMap.java +++ b/src/main/java/net/helenus/mapping/value/ValueProviderMap.java @@ -19,7 +19,6 @@ import java.util.Collection; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; -import net.helenus.core.reflect.Drafted; import net.helenus.mapping.HelenusEntity; import net.helenus.mapping.HelenusProperty; import net.helenus.support.HelenusMappingException; @@ -35,7 +34,7 @@ public final class ValueProviderMap implements Map { this.source = source; this.valueProvider = valueProvider; this.entity = entity; - this.immutable = entity.getMappingInterface().isAssignableFrom(Drafted.class); + this.immutable = entity.isDraftable(); } private static void throwShouldNeverCall(String methodName) { diff --git a/src/test/java/net/helenus/test/integration/core/draft/Inventory.java b/src/test/java/net/helenus/test/integration/core/draft/Inventory.java index 2c4f397..23ec43f 100644 --- a/src/test/java/net/helenus/test/integration/core/draft/Inventory.java +++ b/src/test/java/net/helenus/test/integration/core/draft/Inventory.java @@ -9,7 +9,7 @@ import net.helenus.core.reflect.MapExportable; import net.helenus.mapping.annotation.*; @Table -public interface Inventory extends Entity, Drafted { +public interface Inventory extends Entity { static Inventory inventory = Helenus.dsl(Inventory.class); @@ -38,7 +38,7 @@ public interface Inventory extends Entity, Drafted { return new Draft(this); } - class Draft extends AbstractAuditedEntityDraft { + class Draft extends AbstractAuditedEntityDraft implements Drafted { // Entity/Draft pattern-enabling methods: Draft(UUID id) { diff --git a/src/test/java/net/helenus/test/integration/core/draft/Supply.java b/src/test/java/net/helenus/test/integration/core/draft/Supply.java index 7a28ad2..3b382cc 100644 --- a/src/test/java/net/helenus/test/integration/core/draft/Supply.java +++ b/src/test/java/net/helenus/test/integration/core/draft/Supply.java @@ -15,7 +15,7 @@ import net.helenus.mapping.annotation.*; @Table @Cacheable -public interface Supply extends Entity, Drafted { +public interface Supply extends Entity { static Supply supply = Helenus.dsl(Supply.class); @@ -52,8 +52,7 @@ public interface Supply extends Entity, Drafted { return new Draft(this); } - class Draft extends AbstractEntityDraft { - + class Draft extends AbstractEntityDraft implements Drafted { // Entity/Draft pattern-enabling methods: Draft(String region) { super(null); diff --git a/src/test/java/net/helenus/test/integration/core/unitofwork/UnitOfWorkTest.java b/src/test/java/net/helenus/test/integration/core/unitofwork/UnitOfWorkTest.java index 830732a..fee366a 100644 --- a/src/test/java/net/helenus/test/integration/core/unitofwork/UnitOfWorkTest.java +++ b/src/test/java/net/helenus/test/integration/core/unitofwork/UnitOfWorkTest.java @@ -19,6 +19,7 @@ import static net.helenus.core.Query.eq; import com.datastax.driver.core.ConsistencyLevel; import com.datastax.driver.core.utils.UUIDs; +import java.io.Serializable; import java.util.Date; import java.util.UUID; import net.bytebuddy.utility.RandomString; @@ -38,7 +39,7 @@ import org.junit.Test; @Table @Cacheable -interface Widget extends Entity { +interface Widget extends Entity, Serializable { @PartitionKey UUID id(); diff --git a/src/test/java/net/helenus/test/unit/core/dsl/Account.java b/src/test/java/net/helenus/test/unit/core/dsl/Account.java index 626363f..06d7ba5 100644 --- a/src/test/java/net/helenus/test/unit/core/dsl/Account.java +++ b/src/test/java/net/helenus/test/unit/core/dsl/Account.java @@ -39,7 +39,7 @@ public interface Account { return new Draft(); } - class Draft implements Drafted { // TODO + class Draft implements Drafted { @Override public Set mutated() { @@ -47,7 +47,7 @@ public interface Account { } @Override - public Object build() { + public Account build() { return null; }