From 1eaeaa0e9d416c56b142e481209fa63a084c7ae9 Mon Sep 17 00:00:00 2001
From: Oliver Gierke <ogierke@pivotal.io>
Date: Thu, 13 Aug 2015 09:38:32 +0200
Subject: [PATCH] DATAJPA-775 - Improvements to Spring Data @Version detection.

Moved the detection of an invalidly used @Version to the verification phase of the JpaPersistentEntity so that the MappingContext doesn't even bootstrap if a misconfiguration is detected.

The previous approach would've detected the invalid stage at access time which might actually occur too late.

Original pull request: #153.
---
 .../jpa/mapping/JpaPersistentEntityImpl.java  | 30 +++++++++++-
 .../mapping/JpaPersistentPropertyImpl.java    | 15 ++----
 .../JpaMetamodelMappingContextUnitTests.java  | 49 +++++++++++++++++++
 .../JpaPersistentPropertyImplUnitTests.java   |  9 ----
 4 files changed, 80 insertions(+), 23 deletions(-)
 create mode 100644 src/test/java/org/springframework/data/jpa/mapping/JpaMetamodelMappingContextUnitTests.java

diff --git a/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentEntityImpl.java b/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentEntityImpl.java
index 7f2b2489f..65dd245fc 100644
--- a/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentEntityImpl.java
+++ b/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentEntityImpl.java
@@ -19,6 +19,7 @@ import java.util.Comparator;
 
 import javax.persistence.metamodel.Metamodel;
 
+import org.springframework.data.annotation.Version;
 import org.springframework.data.jpa.provider.PersistenceProvider;
 import org.springframework.data.jpa.provider.ProxyIdAccessor;
 import org.springframework.data.mapping.IdentifierAccessor;
@@ -31,10 +32,15 @@ import org.springframework.util.Assert;
  * Implementation of {@link JpaPersistentEntity}.
  * 
  * @author Oliver Gierke
+ * @author Greg Turnquist
  * @since 1.3
  */
-class JpaPersistentEntityImpl<T> extends BasicPersistentEntity<T, JpaPersistentProperty> implements
-		JpaPersistentEntity<T> {
+class JpaPersistentEntityImpl<T> extends BasicPersistentEntity<T, JpaPersistentProperty>
+		implements JpaPersistentEntity<T> {
+
+	private static final String INVALID_VERSION_ANNOTATION = "%s is annotated with "
+			+ org.springframework.data.annotation.Version.class.getName() + " but needs to use "
+			+ javax.persistence.Version.class.getName() + " to trigger optimistic locking correctly!";
 
 	private final ProxyIdAccessor proxyIdAccessor;
 
@@ -70,6 +76,26 @@ class JpaPersistentEntityImpl<T> extends BasicPersistentEntity<T, JpaPersistentP
 		return new JpaProxyAwareIdentifierAccessor(this, bean, proxyIdAccessor);
 	}
 
+	/* 
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mapping.model.BasicPersistentEntity#verify()
+	 */
+	@Override
+	public void verify() {
+
+		super.verify();
+
+		JpaPersistentProperty versionProperty = getVersionProperty();
+
+		if (versionProperty == null) {
+			return;
+		}
+
+		if (versionProperty.isAnnotationPresent(Version.class)) {
+			throw new IllegalArgumentException(String.format(INVALID_VERSION_ANNOTATION, versionProperty));
+		}
+	}
+
 	/**
 	 * {@link IdentifierAccessor} that tries to use a {@link ProxyIdAccessor} for id access to potentially avoid the
 	 * initialization of JPA proxies. We're falling back to the default behavior of {@link IdPropertyIdentifierAccessor}
diff --git a/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImpl.java b/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImpl.java
index 7917de4c2..dda5cbb2a 100644
--- a/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImpl.java
+++ b/src/main/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImpl.java
@@ -211,21 +211,12 @@ class JpaPersistentPropertyImpl extends AnnotationBasedPersistentProperty<JpaPer
 		return usePropertyAccess != null ? usePropertyAccess : super.usePropertyAccess();
 	}
 
-	/**
-	 * Lookup if the property is versioned. In the event it's versioned using Spring Data Commons and NOT JPA,
-	 * fail fast with a detailed error message.
-	 *
-	 * @return
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mapping.model.AnnotationBasedPersistentProperty#isVersionProperty()
 	 */
 	@Override
 	public boolean isVersionProperty() {
-
-		if (super.isVersionProperty() && !isAnnotationPresent(Version.class)) {
-			throw new IllegalArgumentException(this.owner.getName() + "." + this.getName() + " is annotated with " +
-					org.springframework.data.annotation.Version.class.getCanonicalName() + ". With Spring Data JPA, use " +
-					Version.class.getCanonicalName() + ".");
-		}
-
 		return isAnnotationPresent(Version.class);
 	}
 
diff --git a/src/test/java/org/springframework/data/jpa/mapping/JpaMetamodelMappingContextUnitTests.java b/src/test/java/org/springframework/data/jpa/mapping/JpaMetamodelMappingContextUnitTests.java
new file mode 100644
index 000000000..9f9207934
--- /dev/null
+++ b/src/test/java/org/springframework/data/jpa/mapping/JpaMetamodelMappingContextUnitTests.java
@@ -0,0 +1,49 @@
+/*
+ * Copyright 2015 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.jpa.mapping;
+
+import static org.mockito.Mockito.*;
+
+import java.util.Collections;
+
+import javax.persistence.metamodel.Metamodel;
+
+import org.junit.Test;
+import org.springframework.data.annotation.Version;
+
+/**
+ * Unit tests for {@link JpaMetamodelMappingContext}.
+ * 
+ * @author Oliver Gierke
+ */
+public class JpaMetamodelMappingContextUnitTests {
+
+	/**
+	 * @see DATAJPA-775
+	 */
+	@Test
+	public void jpaPersistentEntityRejectsSprignDataAtVersionAnnotation() {
+
+		Metamodel metamodel = mock(Metamodel.class);
+
+		JpaMetamodelMappingContext context = new JpaMetamodelMappingContext(Collections.singleton(metamodel));
+		context.getPersistentEntity(Sample.class);
+	}
+
+	static class Sample {
+		@Version Long version;
+	}
+}
diff --git a/src/test/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImplUnitTests.java b/src/test/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImplUnitTests.java
index 00963f60c..bc2312021 100644
--- a/src/test/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImplUnitTests.java
+++ b/src/test/java/org/springframework/data/jpa/mapping/JpaPersistentPropertyImplUnitTests.java
@@ -158,15 +158,6 @@ public class JpaPersistentPropertyImplUnitTests {
 		assertThat(getProperty(JpaVersioned.class, "version").isVersionProperty(), is(true));
 	}
 
-	/**
-	 * @see DATAJPA-605
-	 * @see DATAJPA-775
-	 */
-	@Test(expected = IllegalArgumentException.class)
-	public void detectsSpringDataVersionAnnotation() {
-		getProperty(SpringDataVersioned.class, "version");
-	}
-
 	/**
 	 * @see DATAJPA-664
 	 */