Consistency checker does not log errors when one of the objects compa…

…red is null
lightblue-platform · Mar 22, 2016 · a9b4d50 · a9b4d50
1 parent ad0e41d
commit a9b4d50
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 10 deletions.
diff --git a/facade/src/main/java/com/redhat/lightblue/migrator/facade/ConsistencyChecker.java b/facade/src/main/java/com/redhat/lightblue/migrator/facade/ConsistencyChecker.java
@@ -160,7 +160,6 @@ protected boolean checkConsistency(Object o1, Object o2) {
      */
     public boolean checkConsistency(final Object legacyEntity, final Object lightblueEntity, final String methodName, MethodCallStringifier callToLogInCaseOfInconsistency) {
 
-        Timer ti = new Timer("initial");
         if (legacyEntity==null&&lightblueEntity==null) {
             return true;
         }
@@ -194,7 +193,12 @@ public boolean checkConsistency(final Object legacyEntity, final Object lightblu
             // it's slow, but produces nice diffs
             String legacyJsonStr =  objectMapper.writeValueAsString(legacyEntity);
             String lightblueJsonStr =  objectMapper.writeValueAsString(lightblueEntity);
-            logInconsistencyUsingJSONCompare(Thread.currentThread().getName(), legacyJsonStr, lightblueJsonStr, callToLogInCaseOfInconsistency, Boolean.valueOf(System.getProperty("lightblue.facade.consistencyChecker.blocking", "false")));
+
+            if ("null".equals(legacyJsonStr) || "null".equals(lightblueJsonStr)) {
+                logInconsistency(Thread.currentThread().getName(), callToLogInCaseOfInconsistency.toString(), legacyJsonStr, lightblueJsonStr, "One object is null and the other isn't");
+            } else {
+                logInconsistencyUsingJSONCompare(Thread.currentThread().getName(), legacyJsonStr, lightblueJsonStr, callToLogInCaseOfInconsistency, Boolean.valueOf(System.getProperty("lightblue.facade.consistencyChecker.blocking", "false")));
+            }
 
             // inconsistent
             return false;
@@ -212,4 +216,8 @@ public void setMaxInconsistencyLogLength(int length) {
     public void setLogResponseDataEnabled(boolean logResponseDataEnabled) {
         this.logResponseDataEnabled = logResponseDataEnabled;
     }
+
+    void setInconsistencyLog(Logger inconsistencyLog) {
+        this.inconsistencyLog = inconsistencyLog;
+    }
 }
diff --git a/facade/src/test/java/com/redhat/lightblue/migrator/facade/ConsistencyCheckTest.java b/facade/src/test/java/com/redhat/lightblue/migrator/facade/ConsistencyCheckTest.java
@@ -5,28 +5,33 @@
 import java.util.Arrays;
 import java.util.Date;
 
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.runners.MockitoJUnitRunner;
+import org.slf4j.Logger;
 import org.togglz.junit.TogglzRule;
 
 import com.redhat.lightblue.migrator.facade.model.Country;
 import com.redhat.lightblue.migrator.facade.model.CountryInCountry;
 import com.redhat.lightblue.migrator.facade.model.CountryWithBigDecimal;
 import com.redhat.lightblue.migrator.facade.model.CountryWithDate;
 import com.redhat.lightblue.migrator.facade.model.ExtendedCountry;
-import com.redhat.lightblue.migrator.facade.model.Person;
 import com.redhat.lightblue.migrator.facade.model.VeryExtendedCountry;
 import com.redhat.lightblue.migrator.facade.sharedstore.SharedStoreSetter;
 import com.redhat.lightblue.migrator.features.LightblueMigrationFeatures;
 
 @RunWith(MockitoJUnitRunner.class)
 public class ConsistencyCheckTest {
 
+    @Mock
+    private Logger inconsistencyLog;
+
     @Rule
     public TogglzRule togglzRule = TogglzRule.allDisabled(LightblueMigrationFeatures.class);
 
@@ -37,6 +42,13 @@ public class ConsistencyCheckTest {
     @Before
     public void setup() throws InstantiationException, IllegalAccessException {
         consistencyChecker = new ConsistencyChecker(CountryDAO.class.getSimpleName());
+        consistencyChecker.setInconsistencyLog(inconsistencyLog);
+    }
+
+    @After
+    public void after() {
+        // no errors logged to inconsistency log
+        Mockito.verify(inconsistencyLog, Mockito.never()).error(Mockito.anyString());
     }
 
     @Test
@@ -52,9 +64,10 @@ public void testConsistencyWithIgnoredFiled() {
 
     @Test
     public void testConsistencyWithNull() {
-        Assert.assertTrue(consistencyChecker.checkConsistency(null, null));
+        Assert.assertTrue(consistencyChecker.checkConsistency(null, null)); // no log
         Assert.assertFalse(consistencyChecker.checkConsistency(null, new Country()));
         Assert.assertFalse(consistencyChecker.checkConsistency(new Country(), null));
+        Mockito.verify(inconsistencyLog, Mockito.times(2)).warn(Mockito.anyString());
 
         Country c1 = new Country(1l, null);
         Country c2 = new Country(1l, null);
@@ -64,6 +77,9 @@ public void testConsistencyWithNull() {
         Assert.assertTrue(consistencyChecker.checkConsistency(null, null));
         Assert.assertFalse(consistencyChecker.checkConsistency(c1, null));
         Assert.assertFalse(consistencyChecker.checkConsistency(null, c1));
+
+        // 4 inconsistencies means 4 warnings
+        Mockito.verify(inconsistencyLog, Mockito.times(4)).warn(Mockito.anyString());
     }
 
     @Test

diff --git a/facade/src/test/java/com/redhat/lightblue/migrator/facade/ConsistencyLoggerTest.java b/facade/src/test/java/com/redhat/lightblue/migrator/facade/ConsistencyLoggerTest.java
@@ -11,13 +11,15 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Captor;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
+import org.mockito.Mockito;
 import org.mockito.runners.MockitoJUnitRunner;
 import org.slf4j.Logger;
 
@@ -28,7 +30,7 @@
 public class ConsistencyLoggerTest {
 
     @Mock
-    private Logger inconsitencyLog;
+    private Logger inconsistencyLog;
 
     @Mock
     private Logger hugeInconsistencyLog;
@@ -47,6 +49,12 @@ public void setup() throws InstantiationException, IllegalAccessException {
         initMocks(this);
     }
 
+    @After
+    public void after() {
+        // no errors logged to inconsistency log
+        Mockito.verify(inconsistencyLog, Mockito.never()).error(Mockito.anyString());
+    }
+
     private List<String> getDummyMessage(String prefix, int maxBytes) throws JsonProcessingException {
 
         int count = maxBytes / 12; // one country is ~12 bytes when its transformed to json
@@ -72,7 +80,7 @@ public void logTest_logResponsesEnabled_logLessThanLogLimit() throws Exception {
         List<String> lightblue = getDummyMessage("Test", 12);
         boolean result = consistencyChecker.checkConsistency(legacy, lightblue, "testMethod", new LazyMethodCallStringifier("testMethod(param1, param2)"));
         assertFalse(result);
-        verify(inconsitencyLog).warn(logStmt.capture());
+        verify(inconsistencyLog).warn(logStmt.capture());
         assertEquals("[main] Inconsistency found in CountryDAO.testMethod(param1, param2) - diff: []: Expected 2 values but got 1 - legacyJson: [\"Test10000\",\"Test10001\"], lightblueJson: [\"Test10000\"]", logStmt.getValue());
         verify(hugeInconsistencyLog, never()).debug(anyString());
     }
@@ -90,7 +98,7 @@ public void logTest_logResponsesEnabled_logGreaterThanLogLimit() throws Exceptio
         List<String> lightblue = getDummyMessage("Test", 24);
         boolean result = consistencyChecker.checkConsistency(legacy, lightblue, "testMethod", new LazyMethodCallStringifier("testMethod(param1, param2)"));
         assertFalse(result);
-        verify(inconsitencyLog).warn(logStmt.capture());
+        verify(inconsistencyLog).warn(logStmt.capture());
         assertEquals("[main] Inconsistency found in CountryDAO.testMethod(param1, param2) - diff: []: Expected 3 values but got 2", logStmt.getValue());
         verify(hugeInconsistencyLog).debug(inconsistencyLogStmt.capture());
         assertEquals("[main] Inconsistency found in CountryDAO.testMethod(param1, param2) - diff: []: Expected 3 values but got 2 - legacyJson: [\"Test10000\",\"Test10001\",\"Test10002\"], lightblueJson: [\"Test10000\",\"Test10001\"]", inconsistencyLogStmt.getValue());
@@ -109,7 +117,7 @@ public void logTest_logResponsesEnabled_diffGreaterThanLogLimit() throws Excepti
         List<String> lightblue = getDummyMessage("Fooo", 40);
         boolean result = consistencyChecker.checkConsistency(legacy, lightblue, "testMethod", new LazyMethodCallStringifier("testMethod(param1, param2)"));
         assertFalse(result);
-        verify(inconsitencyLog).warn(logStmt.capture());
+        verify(inconsistencyLog).warn(logStmt.capture());
         assertEquals("[main] Inconsistency found in CountryDAO.testMethod(param1, param2) - payload and diff is greater than 177 bytes!", logStmt.getValue());
         verify(hugeInconsistencyLog).debug(inconsistencyLogStmt.capture());
         assertTrue(inconsistencyLogStmt.getValue().contains("diff"));
@@ -130,7 +138,7 @@ public void logTest_logResponsesDisabled_logLessThanLogLimit() throws Exception
         List<String> lightblue = getDummyMessage("Test", 12);
         boolean result = consistencyChecker.checkConsistency(legacy, lightblue, "testMethod", new LazyMethodCallStringifier("testMethod(param1, param2)"));
         assertFalse(result);
-        verify(inconsitencyLog).warn(logStmt.capture());
+        verify(inconsistencyLog).warn(logStmt.capture());
         assertEquals("[main] Inconsistency found in CountryDAO.testMethod(param1, param2) - diff: []: Expected 2 values but got 1", logStmt.getValue());
         verify(hugeInconsistencyLog).debug(inconsistencyLogStmt.capture());
         assertEquals("[main] Inconsistency found in CountryDAO.testMethod(param1, param2) - diff: []: Expected 2 values but got 1 - legacyJson: [\"Test10000\",\"Test10001\"], lightblueJson: [\"Test10000\"]", inconsistencyLogStmt.getValue());
@@ -149,7 +157,7 @@ public void logTest_logResponsesDisabled_diffGreaterThanLogLimit() throws Except
         List<String> lightblue = getDummyMessage("Fooo", 40);
         boolean result = consistencyChecker.checkConsistency(legacy, lightblue, "testMethod", new LazyMethodCallStringifier("testMethod(param1, param2)"));
         assertFalse(result);
-        verify(inconsitencyLog).warn(logStmt.capture());
+        verify(inconsistencyLog).warn(logStmt.capture());
         assertEquals("[main] Inconsistency found in CountryDAO.testMethod(param1, param2) - diff is greater than 177 bytes!", logStmt.getValue());
         verify(hugeInconsistencyLog).debug(inconsistencyLogStmt.capture());
         assertTrue(inconsistencyLogStmt.getValue().contains("diff"));