DATACASS-367 - Drop Serializable requirement on Id types.

Id types for entities are no longer required to implement Serializable. This requirement originates from ancient ORM patterns and had never a technical requirements for Cassandra. Id types are required to map either directly or convert to a Cassandra-supported data type.

spring-data-cassandra/src/main/java/org/springframework/data/cassandra/convert/MappingCassandraConverter.java

@@ -19,7 +19,6 @@ import static org.springframework.data.cassandra.repository.support.BasicMapId.*
 
 import lombok.AllArgsConstructor;
 
-import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -540,7 +539,7 @@ public class MappingCassandraConverter extends AbstractCassandraConverter
 		entity.getPersistentProperties() //
 				.filter(CassandraPersistentProperty::isPrimaryKeyColumn) //
 				.forEach(property -> {
-					id.with(property.getName(), (Serializable) getWriteValue(property, accessor).orElse(null));
+					id.with(property.getName(), getWriteValue(property, accessor).orElse(null));
 				});
 
 		return id;

spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/MapId.java

@@ -15,7 +15,6 @@
  */
 package org.springframework.data.cassandra.repository;
 
-import java.io.Serializable;
 import java.util.Map;
 
 /**
@@ -25,7 +24,7 @@ import java.util.Map;
  * @author Matthew T. Adams
  * @author Mark Paluch
  */
-public interface MapId extends Serializable, Map<String, Object> {
+public interface MapId extends Map<String, Object> {
 
 	/**
 	 * Builder method that adds the value for the named property, then returns {@code this}.

spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/IdInterfaceException.java

@@ -1,13 +1,36 @@
+/*
+ * Copyright 2017 the original author or authors.
+ *
+ * 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 org.springframework.data.cassandra.repository.support;
 
 import java.lang.reflect.Method;
 
-public class IdInterfaceException extends RuntimeException {
+import org.springframework.data.mapping.model.MappingException;
+
+/**
+ * Exception thrown on incorrect mapping of an Id interface.
+ *
+ * @author Matthew T. Adams
+ * @author Mark Paluch
+ */
+public class IdInterfaceException extends MappingException {
 
 	private static final long serialVersionUID = -1635695314254522703L;
 
-	String idInterfaceName;
-	String method;
+	private final String idInterfaceName;
+	private final String method;
 
 	public IdInterfaceException(Class<?> idInterface, Method method, String message) {
 		this(idInterface.getClass().getName(), method == null ? null : method.toString(), message);

spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/IdInterfaceExceptions.java

@@ -16,23 +16,43 @@
 package org.springframework.data.cassandra.repository.support;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.LinkedList;
 import java.util.stream.Collectors;
 
+import org.springframework.data.mapping.model.MappingException;
+import org.springframework.util.Assert;
+
 /**
  * Aggregator of multiple violations for convenience when verifying id interfaces. This allows the framework to
  * communicate all errors at once, rather than one at a time.
  *
  * @author Matthew T. Adams
+ * @author Mark Paluch
  */
 @SuppressWarnings("serial")
-public class IdInterfaceExceptions extends RuntimeException {
+public class IdInterfaceExceptions extends MappingException {
 
-	Collection<IdInterfaceException> exceptions = new LinkedList<>();
-	String idInterfaceName;
+	private final Collection<MappingException> exceptions;
+	private final String className;
 
-	public IdInterfaceExceptions(Class<?> idInterface) {
-		this.idInterfaceName = idInterface.getClass().getName();
+	/**
+	 * Create a new {@link IdInterfaceExceptions} for the given {@code idInterfaceClass} and exceptions.
+	 *
+	 * @param idInterfaceClass must not be {@literal null}.
+	 * @param exceptions must not be {@literal null}.
+	 * @since 2.0
+	 */
+	public IdInterfaceExceptions(Class<?> idInterfaceClass, Collection<MappingException> exceptions) {
+
+		super(String.format("Mapping Exceptions for %s", idInterfaceClass.getName()));
+
+		Assert.notNull(idInterfaceClass, "CassandraPersistentEntity must not be null");
+
+		this.exceptions = Collections.unmodifiableCollection(new LinkedList<>(exceptions));
+		this.className = idInterfaceClass.getName();
+
+		this.exceptions.forEach(this::addSuppressed);
 	}
 
 	public void add(IdInterfaceException e) {
@@ -42,7 +62,7 @@ public class IdInterfaceExceptions extends RuntimeException {
 	/**
 	 * Returns a list of the {@link IdInterfaceException}s aggregated within.
 	 */
-	public Collection<IdInterfaceException> getExceptions() {
+	public Collection<MappingException> getExceptions() {
 		return exceptions;
 	}
 
@@ -62,14 +82,14 @@ public class IdInterfaceExceptions extends RuntimeException {
 
 	@Override
 	public String getMessage() {
-		StringBuilder builder = new StringBuilder(idInterfaceName).append(":\n");
-		for (IdInterfaceException e : exceptions) {
+		StringBuilder builder = new StringBuilder(className).append(":\n");
+		for (MappingException e : exceptions) {
 			builder.append(e.getMessage()).append("\n");
 		}
 		return builder.toString();
 	}
 
 	public String getIdInterfaceName() {
-		return idInterfaceName;
+		return className;
 	}
 }

spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/IdInterfaceValidator.java

@@ -15,11 +15,17 @@
  */
 package org.springframework.data.cassandra.repository.support;
 
-import java.io.Serializable;
 import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.List;
 
 import org.springframework.data.cassandra.repository.MapId;
+import org.springframework.data.mapping.model.MappingException;
 
+/**
+ * @author Matthew T. Adams
+ * @author Mark Paluch
+ */
 public class IdInterfaceValidator {
 
 	/**
@@ -44,10 +50,8 @@ public class IdInterfaceValidator {
 	 * Id interfaces
 	 * <ul>
 	 * <li>must be an {@code interface}, not a <code>class</code>,</li>
-	 * <li>may extend {@link Serializable},</li>
-	 * <li>may extend {@link MapId},</li>
-	 * <li>must have getter methods that only return a {@link Serializable} type,</li>
-	 * <li>must have setter methods that only take a single {@link Serializable} type,</li>
+	 * <li>may extend {@link MapId} or any other interface,</li>
+	 * <li>must have setter methods that only take a single argument,</li>
 	 * <li>must have setter methods that only return {@code void} or the id interface's type,</li>
 	 * <li>must not define any getter methods with the literal name "get", and</li>
 	 * <li>must not define any setter methods with the literal names "set" or "with".</li>
@@ -60,56 +64,57 @@ public class IdInterfaceValidator {
 	 */
 	public static void validate(Class<?> id) {
 
-		IdInterfaceExceptions x = new IdInterfaceExceptions(id);
+		List<MappingException> exceptions = new ArrayList<>();
 
 		if (!id.isInterface()) {
-			x.add(new IdInterfaceException(id, null, "id type must be an interface"));
-		}
-
-		Class<?>[] interfaces = id.getInterfaces();
-		if (interfaces.length > 2 || ((interfaces.length == 1
-				&& !(interfaces[0].equals(Serializable.class) || interfaces[0].equals(MapId.class))))) {
-			x.add(new IdInterfaceException(id, null, "id type may only extend Serializable and/or MapId"));
+			exceptions.add(new IdInterfaceException(id, null, "Id type must be an interface"));
 		}
 
 		for (Method m : id.getDeclaredMethods()) {
+
 			Class<?>[] args = m.getParameterTypes();
 			String name = m.getName();
-			Class<?> ret = m.getReturnType();
+			Class<?> returnType = m.getReturnType();
+
 			switch (args.length) {
 				case 0: // then getter
+
 					if (name.startsWith("get") && name.length() == 3) {
-						x.add(new IdInterfaceException(id, m, "getter methods must have a property name following 'get' prefix"));
+						exceptions
+								.add(new IdInterfaceException(id, m, "Getter method must have a property name following 'get' prefix"));
 					}
-					if (!Serializable.class.isAssignableFrom(ret)) {
-						x.add(new IdInterfaceException(id, m, "getter methods must return Serializable types"));
+
+					if (Void.TYPE.isAssignableFrom(returnType) || Void.class.isAssignableFrom(returnType)) {
+						exceptions.add(new IdInterfaceException(id, m, "Getter method must return a value"));
 					}
+
 					break;
 				case 1: // then setter
+
 					if (name.startsWith("set") && name.length() == 3) {
-						x.add(new IdInterfaceException(id, m, "setter methods must have a property name following 'set' prefix"));
+						exceptions
+								.add(new IdInterfaceException(id, m, "Setter method must have a property name following 'set' prefix"));
 					}
+
 					if (name.startsWith("with") && name.length() == 4) {
-						x.add(new IdInterfaceException(id, m, "setter methods must have a property name following 'with' prefix"));
+						exceptions.add(
+								new IdInterfaceException(id, m, "Setter method must have a property name following 'with' prefix"));
 					}
-					if (!void.class.equals(ret) && !id.equals(ret)) {
-						x.add(new IdInterfaceException(id, m,
-								"setter methods not returning void may only return the same type as their id interface"));
-					}
-					Class<?> arg = args[0];
-					if (!Serializable.class.isAssignableFrom(arg)) {
-						x.add(new IdInterfaceException(id, m, "setter methods must take exactly one Serializable type"));
+
+					if (!Void.TYPE.isAssignableFrom(returnType) && !Void.class.equals(returnType) && !id.equals(returnType)) {
+						exceptions.add(new IdInterfaceException(id, m,
+								"Setter method not returning void may only return the same type as their id interface"));
 					}
 					break;
 				default:
-					x.add(new IdInterfaceException(id, m,
-							"id interface methods may only take zero parameters for a getter or one parameter for a setter; found "
+					exceptions.add(new IdInterfaceException(id, m,
+							"Id interface methods may only take zero parameters for a getter or one parameter for a setter; found "
 									+ args.length));
 			}
 		}
 
-		if (x.getCount() > 0) {
-			throw x;
+		if (!exceptions.isEmpty()) {
+			throw new IdInterfaceExceptions(id, exceptions);
 		}
 	}
 }

spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/MapIdFactory.java

@@ -15,11 +15,14 @@
  */
 package org.springframework.data.cassandra.repository.support;
 
-import java.io.Serializable;
 import java.lang.reflect.Proxy;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
 
 import org.springframework.data.cassandra.repository.MapId;
 import org.springframework.util.Assert;
+import org.springframework.util.ClassUtils;
 
 /**
  * Factory class for producing implementations of given id interfaces. For restrictions on id interfaces definitions,
@@ -35,10 +38,10 @@ public class MapIdFactory {
 	/**
 	 * Produces an implementation of the given id interface type using the type's class loader. For restrictions on id
 	 * interfaces definitions, see {@link IdInterfaceValidator#validate(Class)}. Returns an implementation of the given
-	 * interface that also implements {@link MapId} and {@link Serializable}, so it can be cast as such if necessary.
+	 * interface that also implements {@link MapId}, so it can be cast as such if necessary.
 	 *
 	 * @param idInterface The type of the id interface.
-	 * @return An implementation of the given interface that also implements {@link MapId} and {@link Serializable}.
+	 * @return An implementation of the given interface that also implements {@link MapId}.
 	 * @see IdInterfaceValidator#validate(Class)
 	 */
 	public static <T> T id(Class<T> idInterface) {
@@ -50,10 +53,10 @@ public class MapIdFactory {
 	/**
 	 * Produces an implementation of the given class loader. For restrictions on id interfaces definitions, see
 	 * {@link IdInterfaceValidator#validate(Class)}. Returns an implementation of the given interface that also implements
-	 * {@link MapId} and {@link Serializable}, so it can be cast as such if necessary.
+	 * {@link MapId}, so it can be cast as such if necessary.
 	 *
 	 * @param idInterface The type of the id interface.
-	 * @return An implementation of the given interface that also implements {@link MapId} and {@link Serializable}.
+	 * @return An implementation of the given interface that also implements {@link MapId}.
 	 * @see IdInterfaceValidator#validate(Class)
 	 */
 	public static <T> T id(Class<T> idInterface, ClassLoader loader) {
@@ -63,18 +66,13 @@ public class MapIdFactory {
 
 		IdInterfaceValidator.validate(idInterface);
 
-		Class<?>[] interfaces = idInterface.getInterfaces();
-		switch (interfaces.length) {
-			case 0:
-				interfaces = new Class<?>[] { idInterface, MapId.class, Serializable.class };
-				break;
-			case 1:
-				Class<?> other = interfaces[0].equals(Serializable.class) ? MapId.class : Serializable.class;
-				interfaces = new Class<?>[] { idInterface, interfaces[0], other };
-				break;
-			default:
-				interfaces = new Class<?>[] { idInterface, interfaces[0], interfaces[1] };
-		}
-		return (T) Proxy.newProxyInstance(loader, interfaces, new MapIdProxyDelegate(idInterface));
+		Class<?>[] idInterfaces = ClassUtils.getAllInterfacesForClass(idInterface);
+		Set<Class<?>> proxyInterfaces = new HashSet<>(idInterfaces.length + 1, 1);
+
+		proxyInterfaces.add(MapId.class);
+		proxyInterfaces.addAll(Arrays.asList(idInterfaces));
+
+		return (T) Proxy.newProxyInstance(loader, proxyInterfaces.toArray(new Class[proxyInterfaces.size()]),
+				new MapIdProxyDelegate(idInterface));
 	}
 }

spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/support/MapIdProxyDelegate.java

@@ -15,7 +15,6 @@
  */
 package org.springframework.data.cassandra.repository.support;
 
-import java.io.Serializable;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
 import java.util.Arrays;
@@ -110,11 +109,8 @@ class MapIdProxyDelegate implements InvocationHandler {
 			delegate.put(name, null);
 			return;
 		}
-		if (!(value instanceof Serializable)) {
-			throw new IllegalArgumentException(
-					String.format("Given object [%s] must implement %s", value, Serializable.class.getName()));
-		}
-		delegate.put(name, (Serializable) value);
+
+		delegate.put(name, value);
 	}
 }
 

spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/support/MapIdFactoryUnitTests.java

@@ -30,6 +30,7 @@ import org.springframework.data.cassandra.repository.MapId;
  * Unit tests for {@link MapIdFactory}.
  *
  * @author Matthew T. Adams
+ * @author Mark Paluch
  */
 public class MapIdFactoryUnitTests {
 
@@ -209,8 +210,6 @@ public class MapIdFactoryUnitTests {
 
 	interface Foo {}
 
-	interface IdExtendingNotMapId extends Foo {}
-
 	interface LiteralGet {
 		String get();
 	}
@@ -223,10 +222,6 @@ public class MapIdFactoryUnitTests {
 		void string();
 	}
 
-	interface GetReturningNonSerializable {
-		Object getFoo();
-	}
-
 	interface MethodWithMoreThanOneArgument {
 		void foo(Object a, Object b);
 	}
@@ -251,30 +246,18 @@ public class MapIdFactoryUnitTests {
 		String withString(String s);
 	}
 
-	interface SetterMethodTakingNonSerializable {
-		void string(Object o);
-	}
-
-	interface SetMethodTakingNonSerializable {
-		void string(Object o);
-	}
-
-	interface WithMethodTakingNonSerializable {
-		void string(Object o);
-	}
-
 	@Test
 	public void testUnhappies() {
-		Class<?>[] interfaces = new Class<?>[] { IdClass.class, IdExtendingNotMapId.class, LiteralGet.class,
-				GetterReturningVoid.class, GetReturningVoid.class, GetReturningNonSerializable.class,
-				MethodWithMoreThanOneArgument.class, LiteralSet.class, LiteralWith.class,
+
+		Class<?>[] interfaces = new Class<?>[] { IdClass.class, LiteralGet.class, GetterReturningVoid.class,
+				GetReturningVoid.class, MethodWithMoreThanOneArgument.class, LiteralSet.class, LiteralWith.class,
 				SetterMethodNotReturningVoidOrThis.class, SetMethodNotReturningVoidOrThis.class,
-				WithMethodNotReturningVoidOrThis.class, SetterMethodTakingNonSerializable.class,
-				SetMethodTakingNonSerializable.class, WithMethodTakingNonSerializable.class };
-		for (Class<?> i : interfaces) {
+				WithMethodNotReturningVoidOrThis.class };
+
+		for (Class<?> idInterface : interfaces) {
 			try {
-				validate(i);
-				fail("should've caught IdInterfaceException validating interface " + i);
+				validate(idInterface);
+				fail("should've caught IdInterfaceException validating interface " + idInterface);
 			} catch (IdInterfaceExceptions e) {
 				assertThat(e.getCount()).isEqualTo(1);
 			}