Add safety checks to SolrUpdateEventHandler methods

paulcwarren · Jan 13, 2017 · f188876 · f188876
1 parent a3e66ae
commit f188876
Show file tree

Hide file tree

Showing 2 changed files with 103 additions and 16 deletions.
diff --git a/.../java/internal/org/springframework/content/autoconfigure/solr/SolrUpdateEventHandler.java b/.../java/internal/org/springframework/content/autoconfigure/solr/SolrUpdateEventHandler.java
@@ -33,7 +33,14 @@ public SolrUpdateEventHandler(SolrClient solrCient) {
 
 	@Override
 	protected void onAfterSetContent(Object contentEntity) {
-
+		if (BeanUtils.hasFieldWithAnnotation(contentEntity, ContentId.class) == false) {
+			return;
+		}
+
+		if (BeanUtils.getFieldWithAnnotation(contentEntity, ContentId.class) == null) {
+			return;
+		}
+
 	    ContentStreamUpdateRequest up = new ContentStreamUpdateRequest("/update/extract");
 	    up.addContentStream(new ContentEntityStream(ops, contentEntity));
 	    up.setParam("literal.id", BeanUtils.getFieldWithAnnotation(contentEntity, ContentId.class).toString());
@@ -47,6 +54,28 @@ protected void onAfterSetContent(Object contentEntity) {
 		}
 	}
 
+	@Override
+	protected void onBeforeUnsetContent(Object contentEntity) {
+		if (BeanUtils.hasFieldWithAnnotation(contentEntity, ContentId.class) == false) {
+			return;
+		}
+
+		Object id = BeanUtils.getFieldWithAnnotation(contentEntity, ContentId.class); 
+		if (id == null) {
+			return;
+		}
+
+		try {
+			solrClient.deleteById(id.toString());
+			solrClient.commit();
+		} catch (SolrServerException e) {
+			e.printStackTrace();
+		} catch (IOException e) {
+			e.printStackTrace();
+		}
+	}
+
+
 	public class ContentEntityStream extends ContentStreamBase {
 
 		private ContentOperations ops;

diff --git a/.../internal/org/springframework/content/autoconfigure/solr/SolrUpdateEventHandlerTests.java b/.../internal/org/springframework/content/autoconfigure/solr/SolrUpdateEventHandlerTests.java
@@ -1,28 +1,25 @@
 package internal.org.springframework.content.autoconfigure.solr;
 
-import static org.hamcrest.CoreMatchers.instanceOf;
+import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.BeforeEach;
+import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.Context;
+import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.Describe;
+import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.It;
+import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.FIt;
+import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.JustBeforeEach;
 import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.CoreMatchers.isA;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
-
 import static org.mockito.Matchers.anyObject;
 import static org.mockito.Matchers.anyString;
-import static org.mockito.Matchers.argThat;
-import static org.mockito.Matchers.isNull;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 
 import java.util.UUID;
 
 import org.apache.solr.client.solrj.SolrClient;
-import org.apache.solr.client.solrj.SolrRequest;
-
-import static com.github.paulcwarren.ginkgo4j.Ginkgo4jDSL.*;
-
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Matchers;
 import org.springframework.content.commons.annotations.ContentId;
 import org.springframework.content.commons.annotations.ContentLength;
 import org.springframework.content.commons.annotations.MimeType;
@@ -45,13 +42,13 @@ public class SolrUpdateEventHandlerTests {
 
 	{
 		Describe("SolrUpdateEventHandler", () -> {
+			BeforeEach(() -> {
+				solrClient = mock(SolrClient.class);
+				ops = mock(ContentOperations.class);
+				handler = new SolrUpdateEventHandler(solrClient, ops);
+			});
 			Context("#onAfterSetContent", () -> {
-				BeforeEach(() -> {
-					solrClient = mock(SolrClient.class);
-					ops = mock(ContentOperations.class);
-				});
 				JustBeforeEach(() -> {
-					handler = new SolrUpdateEventHandler(solrClient, ops);
 					try {
 						handler.onAfterSetContent(contentEntity);
 					} catch (Exception e) {
@@ -70,6 +67,64 @@ public class SolrUpdateEventHandlerTests {
 						verify(solrClient).request(anyObject(), anyString());
 					});
 				});
+				Context("given a content entity with a null contentId", () -> {
+					BeforeEach(() -> {
+						contentEntity = new ContentEntity();
+					});
+					It("should call update", () -> {
+						assertThat(e, is(nullValue()));
+						verify(solrClient, never()).request(anyObject(), anyString());
+					});
+				});
+				Context("given a bogus content entity", () -> {
+					BeforeEach(() -> {
+						contentEntity = new NotAContentEntity();
+					});
+					It("", ()->{
+						assertThat(e, is(nullValue()));
+						verify(solrClient, never()).request(anyObject(), anyString());
+					});
+				});
+			});
+			Context("#onBeforeUnsetContent", () -> {
+				JustBeforeEach(() -> {
+					try {
+						handler.onBeforeUnsetContent(contentEntity);
+					} catch (Exception e) {
+						this.e = e;
+					}
+				});
+				Context("given a content entity", () -> {
+					BeforeEach(() -> {
+						contentEntity = new ContentEntity();
+						((ContentEntity)contentEntity).contentId = UUID.randomUUID().toString();
+						((ContentEntity)contentEntity).contentLen = 128L;
+						((ContentEntity)contentEntity).mimeType = "text/plain";
+					});
+					It("should call deleteById", () -> {
+						assertThat(e, is(nullValue()));
+						verify(solrClient).deleteById(((ContentEntity)contentEntity).contentId.toString());
+						verify(solrClient).commit();
+					});
+				});
+				Context("given a content entity with a null contentId", () -> {
+					BeforeEach(() -> {
+						contentEntity = new ContentEntity();
+					});
+					It("should call update", () -> {
+						assertThat(e, is(nullValue()));
+						verify(solrClient, never()).deleteById(anyString());
+					});
+				});
+				Context("given a bogus content entity", () -> {
+					BeforeEach(() -> {
+						contentEntity = new NotAContentEntity();
+					});
+					It("", ()->{
+						assertThat(e, is(nullValue()));
+						verify(solrClient, never()).deleteById(anyString());
+					});
+				});
 			});
 		});
 	}
@@ -80,6 +135,9 @@ public static class ContentEntity {
 		@MimeType public String mimeType;
 	}
 
+	public static class NotAContentEntity {
+	}
+
 	@Test
 	public void test() {
 	}