Skip to content

Commit

Permalink
#132 - review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
akrystian committed Jun 12, 2016
1 parent a505d7c commit 34f3439
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 58 deletions.
Expand Up @@ -40,8 +40,6 @@ public enum JaversExceptionCode {

SET_OF_VO_DIFF_NOT_IMPLEMENTED("diff for Set of ValueObjects is not supported"),

MULTISET_OF_VO_DIFF_NOT_IMPLEMENTED("diff for Multiset of ValueObjects is not supported"),

VALUE_OBJECT_IS_NOT_SUPPORTED_AS_MULTIMAP_KEY("found ValueObject on KEY position in Multimap property '%s'. Please change the key class mapping to Value or Entity"),

GENERIC_TYPE_NOT_PARAMETRIZED(
Expand Down
15 changes: 2 additions & 13 deletions javers-core/src/main/java/org/javers/core/JaversBuilder.java
Expand Up @@ -21,19 +21,8 @@
import org.javers.core.json.typeadapter.change.ChangeTypeAdaptersModule;
import org.javers.core.json.typeadapter.commit.CommitTypeAdaptersModule;
import org.javers.core.json.typeadapter.util.UtilTypeAdapters;
import org.javers.core.metamodel.annotation.DiffIgnore;
import org.javers.core.metamodel.annotation.Entity;
import org.javers.core.metamodel.annotation.TypeName;
import org.javers.core.metamodel.annotation.Value;
import org.javers.core.metamodel.annotation.ValueObject;
import org.javers.core.metamodel.clazz.ClientsClassDefinition;
import org.javers.core.metamodel.clazz.CustomDefinition;
import org.javers.core.metamodel.clazz.EntityDefinition;
import org.javers.core.metamodel.clazz.EntityDefinitionBuilder;
import org.javers.core.metamodel.clazz.IgnoredTypeDefinition;
import org.javers.core.metamodel.clazz.ValueDefinition;
import org.javers.core.metamodel.clazz.ValueObjectDefinition;
import org.javers.core.metamodel.clazz.ValueObjectDefinitionBuilder;
import org.javers.core.metamodel.annotation.*;
import org.javers.core.metamodel.clazz.*;
import org.javers.core.metamodel.object.GlobalIdFactory;
import org.javers.core.metamodel.scanner.ScannerModule;
import org.javers.core.metamodel.type.CustomType;
Expand Down
@@ -1,7 +1,5 @@
package org.javers.core.metamodel.type;

import com.google.common.collect.Multimap;
import com.google.common.collect.Multiset;
import org.javers.common.collections.Primitives;
import org.javers.common.exception.JaversException;
import org.javers.common.exception.JaversExceptionCode;
Expand Down Expand Up @@ -83,10 +81,10 @@ private void registerCoreTypes(){
}

if (ReflectionUtil.isClassPresent("com.google.common.collect.Multiset")){
addType(new MultisetType(Multiset.class));
addType(new MultisetType());
}
if (ReflectionUtil.isClassPresent("com.google.common.collect.Multimap")){
addType(new MultimapType(Multimap.class));
addType(new MultimapType());
}
}

Expand Down
Expand Up @@ -21,8 +21,8 @@
*/
public class MultimapType extends EnumerableType{

public MultimapType(Type baseJavaType){
super(baseJavaType);
public MultimapType(){
super(Multimap.class);
}

@Override
Expand Down
Expand Up @@ -2,8 +2,6 @@

import com.google.common.collect.Multiset;
import com.google.common.collect.Multisets;
import org.javers.common.exception.JaversException;
import org.javers.common.exception.JaversExceptionCode;
import org.javers.core.diff.changetype.container.ContainerElementChange;
import org.javers.core.diff.changetype.container.ValueAdded;
import org.javers.core.diff.changetype.container.ValueRemoved;
Expand All @@ -19,12 +17,9 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.List;

import static org.javers.common.exception.JaversExceptionCode.GENERIC_TYPE_NOT_PARAMETRIZED;

/**
* Compares Multiset.
* <p>
Expand All @@ -43,27 +38,9 @@ public MultisetComparator(TypeMapper typeMapper, GlobalIdFactory globalIdFactory
this.globalIdFactory = globalIdFactory;
}

private boolean isSupportedContainer(Property property){
JaversType propertyType = typeMapper.getPropertyType(property);
if (typeMapper.isValueObject(getItemType(propertyType))){
logger.error("could not diff " + property + ", " +
JaversExceptionCode.MULTISET_OF_VO_DIFF_NOT_IMPLEMENTED.getMessage());
return false;
}
return true;
}

public Type getItemType(JaversType propertyType){
List<Type> actualTypeArguments = propertyType.getActualTypeArguments();
if (actualTypeArguments.size() == 1){
return propertyType.getActualTypeArguments().get(0);
}
throw new JaversException(GENERIC_TYPE_NOT_PARAMETRIZED, propertyType.getBaseJavaType().toString());
}

@Override
public MultisetChange compare(Multiset left, Multiset right, GlobalId affectedId, Property property){
if (!isSupportedContainer(property) || left.equals(right)){
if (left.equals(right)){
return null;
}

Expand Down
Expand Up @@ -9,15 +9,13 @@
import org.javers.core.metamodel.object.OwnerContext;
import org.javers.core.metamodel.type.CollectionType;

import java.lang.reflect.Type;

/**
* @author akrystian
*/
public class MultisetType extends CollectionType{

public MultisetType(Type baseJavaType) {
super(baseJavaType);
public MultisetType() {
super(Multiset.class);
}

@Override
Expand Down
@@ -1,4 +1,5 @@
package org.javers.guavasupport

import com.google.common.collect.HashMultimap
import com.google.common.collect.HashMultiset
import com.google.common.collect.Multimap
Expand All @@ -8,6 +9,7 @@ import spock.lang.Specification
import spock.lang.Unroll

import static org.javers.core.JaversBuilder.javers

/**
* @author akrystian
*/
Expand All @@ -29,11 +31,11 @@ class GuavaAddOnE2ETest extends Specification {
changes.size() == extpectedChanges

where:
leftList | rightList | extpectedChanges
["New York"] | ["Boston"] | 2
["New York"] | ["New York", "New York"] | 1
Collections.emptyList() | ["New York"] | 1
["New York"] | Collections.emptyList() | 1
leftList | rightList | extpectedChanges
["New York"] | ["Boston"] | 2
["New York"] | ["New York", "New York"] | 1
[] | ["New York"] | 1
["New York"] | [] | 1
}

@Unroll
Expand All @@ -50,10 +52,10 @@ class GuavaAddOnE2ETest extends Specification {
diff.changes.size() == 0

where:
leftList | rightList
["New York"] | ["New York"]
Collections.emptyList() | Collections.emptyList()
["New York", "Boston"] | ["Boston", "New York"]
leftList | rightList
["New York"] | ["New York"]
[] | []
["New York", "Boston"] | ["Boston", "New York"]
}

@Unroll
Expand Down

0 comments on commit 34f3439

Please sign in to comment.