Merge pull request #141 from kit-data-manager/135-h2-db-+-local-pidsystem-may-cause-unexpected-errors

Fix 135 "h2 db + local pidsystem may cause unexpected errors"

Author: Pfeil

src/main/java/edu/kit/datamanager/pit/pidsystem/impl/local/PidDatabaseObject.java

@@ -32,7 +32,31 @@ public class PidDatabaseObject {
     @Column(name = "pid")
     private String pid;
 
+    /**
+     * About the column definition we use here:
+     * 
+     * Some databases (known: h2) store collections in encoded strings. This causes
+     * an issue for mid-to-large records. We need to increase the length of stings
+     * in this case:
+     * 
+     * int javax.persistence.Column.length() (Optional) The column length. (Applies
+     * only if a string-valued column is used.) Default: 255
+     * 
+     * The h2 database we use for testing has a CHARACTER VARYING limit of
+     * 1_000_000_000
+     * (http://h2database.com/html/datatypes.html#character_varying_type).
+     * 
+     * Postgres does not have a limit according to
+     * https://www.postgresql.org/docs/current/datatype-character.html, but we
+     * assume it will not use its text datatype for the collection anyway.
+     * 
+     * The limit of MySql seems to be 65_535. SQlite has a upper limit of
+     * 1_000_000_000 (set by default).
+     * 
+     * Our extended tests with the h2 database require a length of ~6500 or more.
+     */
     @ElementCollection(fetch = FetchType.EAGER)
+    @Column(length = 65_535)
     private Map<String, ArrayList<String>> entries = new HashMap<>();
 
     /** For hibernate */

src/test/java/edu/kit/datamanager/pit/RecordTestHelper.java

@@ -0,0 +1,43 @@
+package edu.kit.datamanager.pit;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.stream.IntStream;
+
+import edu.kit.datamanager.pit.domain.PIDRecord;
+import edu.kit.datamanager.pit.pidgeneration.PidSuffixGenerator;
+
+public class RecordTestHelper {
+    /**
+     * Generates a PID record with N attibutes, each having M values. All attributes
+     * and values are PIDs of {@link pidGenerator} prefixed with {@link PID_PREFIX}.
+     * 
+     * @param numAttributes the number of attibutes (called N in the description
+     *                      above)
+     * @param numValues     the number of attributes (called M in the description
+     *                      above)
+     * @return a PID record as configured.
+     */
+    public static PIDRecord getFakePidRecord(
+            final int numAttributes,
+            final int numValues,
+            final String prefix,
+            final PidSuffixGenerator generator)
+    {
+        PIDRecord r = new PIDRecord();
+        r.setPid(generator.generate().get());
+        IntStream.range(0, numAttributes)
+                .mapToObj(i -> generator.generate().getWithPrefix(prefix))
+                .forEach(attribute -> {
+                    IntStream.range(0, numValues)
+                            .mapToObj(i -> generator.generate().getWithPrefix(prefix))
+                            .forEach(value -> r.addEntry(attribute, "name", value));
+                });
+
+        // In theory these could fail if the generator generated a PID twice, but it is very unlikely.
+        assertEquals(numAttributes, r.getPropertyIdentifiers().size());
+        assertEquals(numValues, r.getPropertyValues(r.getPropertyIdentifiers().iterator().next()).length);
+        
+        return r;
+    }
+}

src/test/java/edu/kit/datamanager/pit/pidsystem/IIdentifierSystemTest.java src/test/java/edu/kit/datamanager/pit/pidsystem/IIdentifierSystemQueryTest.java

@@ -21,11 +21,17 @@
 import net.handle.hdllib.HandleException;
 
 /**
- * This test ensures the interface is implemented correctly, with the following limits:
- * - TODO do write tests (update, register, delete PIDs), as this requires authentication or mocking
+ * This test ensures the interface is implemented correctly, with the following
+ * limits:
+ * 
+ * - Only read-tests (write tests are only possible for sandboxed systems on a
+ * regular and automated basis)
  * - TODO do queryByType tests (requires PID with registered type as property)
+ * 
+ * NOTE: The difference to the tests in the web module is that this only tests
+ * the pidsystem without any validation or other functionality.
  */
-public class IIdentifierSystemTest {
+public class IIdentifierSystemQueryTest {
 
     private static Stream<Arguments> implProvider() throws HandleException, IOException {
         HandleProtocolProperties props = new HandleProtocolProperties();

src/test/java/edu/kit/datamanager/pit/pidsystem/IIdentifierSystemWriteTest.java

@@ -0,0 +1,81 @@
+package edu.kit.datamanager.pit.pidsystem;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.io.IOException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.autoconfigure.jdbc.DataSourceProperties;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.test.context.ActiveProfiles;
+import org.springframework.test.context.TestPropertySource;
+
+import edu.kit.datamanager.pit.RecordTestHelper;
+import edu.kit.datamanager.pit.domain.PIDRecord;
+import edu.kit.datamanager.pit.pidgeneration.PidSuffixGenerator;
+import edu.kit.datamanager.pit.pidsystem.impl.local.LocalPidSystem;
+
+/**
+ * Write tests are only possible with sandboxed PID systems like InMemory and
+ * Local.
+ * 
+ * The aim is to test the creation of large PID records.
+ * 
+ * For the Local system, we need the database and therefore a Spring context.
+ */
+// JUnit5 + Spring
+@SpringBootTest
+// Set the in-memory implementation
+@TestPropertySource(
+    locations = "/test/application-test.properties",
+    properties = "pit.pidsystem.implementation=LOCAL"
+)
+@ActiveProfiles("test")
+public class IIdentifierSystemWriteTest {
+
+    @Autowired
+    private LocalPidSystem localPidSystem;
+
+    @Autowired
+    private PidSuffixGenerator pidGenerator;
+
+    private static final String PID_PREFIX = "sandboxed/";
+
+    @Autowired
+    DataSourceProperties dataSourceProperties;
+    
+    @BeforeEach
+    void setup() throws InterruptedException, IOException {
+        // ensure we run on an in-memory DB for testing
+        //assertTrue(dataSourceProperties.determineUrl().contains("mem"));
+        // ensure wiring worked
+        assertNotNull(localPidSystem);
+        
+        // Register just a small record for the database to initialize maybe or so.
+        PIDRecord r = new PIDRecord();
+        r.setPid(pidGenerator.generate().get());
+        String attribute = pidGenerator.generate().getWithPrefix(PID_PREFIX);
+        String value = pidGenerator.generate().getWithPrefix(PID_PREFIX);
+        r.addEntry(attribute, "test", value);
+
+        localPidSystem.registerPID(r);
+    }
+
+    @Test
+    @DisplayName("Testing PID Records with usual/larger size, with the Local PID system (in-memory db).")
+    void testExtensiveRecordWithLocalPidSystem() throws IOException {
+        // as we use an in-memory db for testing, lets not make it too large.
+        int numAttributes = 100;
+        int numValues = 100;
+
+        PIDRecord r = RecordTestHelper.getFakePidRecord(numAttributes, numValues, PID_PREFIX, pidGenerator);
+        assertEquals(numAttributes, r.getPropertyIdentifiers().size());
+        assertEquals(numValues, r.getPropertyValues(r.getPropertyIdentifiers().iterator().next()).length);
+
+        this.localPidSystem.registerPID(r);
+    }
+}

src/test/java/edu/kit/datamanager/pit/pidsystem/impl/local/LocalPidSystemTest.java

@@ -24,7 +24,7 @@
 import edu.kit.datamanager.pit.common.InvalidConfigException;
 import edu.kit.datamanager.pit.domain.PIDRecord;
 import edu.kit.datamanager.pit.domain.TypeDefinition;
-import edu.kit.datamanager.pit.pidsystem.IIdentifierSystemTest;
+import edu.kit.datamanager.pit.pidsystem.IIdentifierSystemQueryTest;
 
 /**
  * This tests the same things as `IIdentifierSystemTest`, but is separated from
@@ -40,7 +40,7 @@
 )
 @ActiveProfiles("test")
 public class LocalPidSystemTest {
-    IIdentifierSystemTest systemTests = new IIdentifierSystemTest();
+    IIdentifierSystemQueryTest systemTests = new IIdentifierSystemQueryTest();
 
     @Autowired
     LocalPidSystem localPidSystem;
@@ -96,8 +96,8 @@ void testAllSystemTests() throws Exception {
         PIDRecord newRec = localPidSystem.queryAllProperties(pid);
         assertEquals(rec, newRec);
 
-        Set<Method> publicMethods = new HashSet<>(Arrays.asList(IIdentifierSystemTest.class.getMethods()));
-        Set<Method> allDirectMethods = new HashSet<>(Arrays.asList(IIdentifierSystemTest.class.getDeclaredMethods()));
+        Set<Method> publicMethods = new HashSet<>(Arrays.asList(IIdentifierSystemQueryTest.class.getMethods()));
+        Set<Method> allDirectMethods = new HashSet<>(Arrays.asList(IIdentifierSystemQueryTest.class.getDeclaredMethods()));
         publicMethods.retainAll(allDirectMethods);
         assertEquals(7, publicMethods.size());
         for (Method test : publicMethods) {

src/test/java/edu/kit/datamanager/pit/web/RestWithInMemoryTest.java

@@ -13,6 +13,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -22,7 +23,11 @@
 import org.springframework.http.MediaType;
 import org.springframework.web.context.WebApplicationContext;
 
+import edu.kit.datamanager.pit.RecordTestHelper;
+import edu.kit.datamanager.pit.configuration.ApplicationProperties;
+import edu.kit.datamanager.pit.configuration.ApplicationProperties.ValidationStrategy;
 import edu.kit.datamanager.pit.domain.PIDRecord;
+import edu.kit.datamanager.pit.pidgeneration.PidSuffixGenerator;
 import edu.kit.datamanager.pit.pidlog.KnownPid;
 import edu.kit.datamanager.pit.pidlog.KnownPidsDao;
 import edu.kit.datamanager.pit.pidsystem.impl.HandleProtocolAdapter;
@@ -69,6 +74,12 @@ public class RestWithInMemoryTest {
     @Autowired
     private WebApplicationContext webApplicationContext;
 
+    @Autowired
+    private PidSuffixGenerator pidGenerator;
+
+    @Autowired
+    private ApplicationProperties appProps;
+
     private MockMvc mockMvc;
 
     private ObjectMapper mapper;
@@ -85,6 +96,7 @@ public void setup() throws Exception {
         this.mockMvc = MockMvcBuilders.webAppContextSetup(this.webApplicationContext).build();
         this.mapper = this.webApplicationContext.getBean("OBJECT_MAPPER_BEAN", ObjectMapper.class);
         this.knownPidsDao.deleteAll();
+        this.appProps.setValidationStrategy(ValidationStrategy.EMBEDDED_STRICT);
     }
 
     @Test
@@ -130,6 +142,27 @@ public void testCreateEmptyRecord() throws Exception {
         assertEquals(0, this.knownPidsDao.count());
     }
 
+    @Test
+    @DisplayName("Testing PID Records with usual/larger size, with the InMemory PID system.")
+    public void testExtensiveRecord() throws Exception {
+        // create mockup of a large record. It contains non-registered PIDs and can not be validated.
+        this.appProps.setValidationStrategy(ValidationStrategy.NONE_DEBUG);
+        // as we use an in-memory data structure, lets not make it too large.
+        int numAttributes = 100;
+        int numValues = 100;
+        assertTrue(numAttributes * numValues > 256);
+        PIDRecord r = RecordTestHelper.getFakePidRecord(numAttributes, numValues, "sandboxed/", pidGenerator);
+        
+        String rJson = ApiMockUtils.serialize(r);
+        ApiMockUtils.registerRecord(
+            this.mockMvc,
+            rJson,
+            MediaType.APPLICATION_JSON_VALUE,
+            MediaType.APPLICATION_JSON_VALUE,
+            MockMvcResultMatchers.status().isCreated()
+        );
+    }
+
     @Test
     public void testNontypeRecord() throws Exception {
         PIDRecord r = new PIDRecord();

src/test/java/edu/kit/datamanager/pit/web/RestWithLocalPidSystemTest.java

@@ -12,6 +12,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -21,13 +22,17 @@
 import org.springframework.http.MediaType;
 import org.springframework.web.context.WebApplicationContext;
 
+import edu.kit.datamanager.pit.RecordTestHelper;
+import edu.kit.datamanager.pit.configuration.ApplicationProperties;
+import edu.kit.datamanager.pit.configuration.ApplicationProperties.ValidationStrategy;
 import edu.kit.datamanager.pit.domain.PIDRecord;
+import edu.kit.datamanager.pit.pidgeneration.PidSuffixGenerator;
 import edu.kit.datamanager.pit.pidlog.KnownPid;
 import edu.kit.datamanager.pit.pidlog.KnownPidsDao;
 import edu.kit.datamanager.pit.pidsystem.impl.HandleProtocolAdapter;
 import edu.kit.datamanager.pit.pidsystem.impl.InMemoryIdentifierSystem;
 import edu.kit.datamanager.pit.pidsystem.impl.local.LocalPidSystem;
-import edu.kit.datamanager.pit.web.impl.TypingRESTResourceImpl;
+import edu.kit.datamanager.pit.pitservice.ITypingService;
 
 // org.springframework.mock is for unit testing
 // Source: https://docs.spring.io/spring-framework/docs/current/reference/html/testing.html
@@ -75,6 +80,15 @@ public class RestWithLocalPidSystemTest {
     @Autowired
     private WebApplicationContext webApplicationContext;
 
+    @Autowired
+    private PidSuffixGenerator pidGenerator;
+
+    @Autowired
+    ITypingService typingService;
+
+    @Autowired
+    ApplicationProperties appProps;
+
     private MockMvc mockMvc;
 
     private ObjectMapper mapper;
@@ -91,6 +105,7 @@ public void setup() throws Exception {
         this.mockMvc = MockMvcBuilders.webAppContextSetup(this.webApplicationContext).build();
         this.mapper = this.webApplicationContext.getBean("OBJECT_MAPPER_BEAN", ObjectMapper.class);
         this.knownPidsDao.deleteAll();
+        this.appProps.setValidationStrategy(ValidationStrategy.EMBEDDED_STRICT);
     }
 
     @Test
@@ -163,6 +178,27 @@ public void testCreateValidRecord() throws Exception {
         kp.getCreated().isBefore(kp.getModified());
     }
 
+    @Test
+    @DisplayName("Testing PID Records with usual/larger size, with the Local PID system (in-memory db).")
+    public void testExtensiveRecord() throws Exception {
+        // create mockup of a large record. It contains non-registered PIDs and can not be validated.
+        this.appProps.setValidationStrategy(ValidationStrategy.NONE_DEBUG);
+        // as we use an in-memory db for testing, lets not make it too large.
+        int numAttributes = 100;
+        int numValues = 100;
+        assertTrue(numAttributes * numValues > 256);
+        PIDRecord r = RecordTestHelper.getFakePidRecord(numAttributes, numValues, "sandboxed/", pidGenerator);
+        
+        String rJson = ApiMockUtils.serialize(r);
+        ApiMockUtils.registerRecord(
+            this.mockMvc,
+            rJson,
+            MediaType.APPLICATION_JSON_VALUE,
+            MediaType.APPLICATION_JSON_VALUE,
+            MockMvcResultMatchers.status().isCreated()
+        );
+    }
+
     @Test
     public void testUpdateRecord() throws Exception {
         PIDRecord record = this.createSomeRecord();