Merge branch 'f-fix-subselect-list' into 'dev'

subselect

See merge request caosdb/caosdb-server!75

CHANGELOG.md

@@ -95,6 +95,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+* Missing handling of list of reference properties in SELECT queries.
 * #51 - name queries (e.g. `FIND ENTITY WITH name = ...`)
 - #27 - star matches slashes (e.g. for `FIND ... STORED AT /*.dat`).
 - #30 - file path cannot be in quotes

src/main/java/org/caosdb/server/database/backend/transaction/RetrieveFullEntity.java

 /*
- * ** header v3.0 This file is a part of the CaosDB Project.
+ * ** header v3.0
+ * This file is a part of the CaosDB Project.
  *
- * Copyright (C) 2018 Research Group Biomedical Physics, Max-Planck-Institute for Dynamics and
- * Self-Organization Göttingen Copyright (C) 2020 Timm Fitschen <t.fitschen@indiscale.com> Copyright
- * (C) 2020 IndiScale GmbH <info@indiscale.com>
+ * Copyright (C) 2018 Research Group Biomedical Physics,
+ *   Max-Planck-Institute for Dynamics and Self-Organization Göttingen
+ * Copyright (C) 2020-2021 IndiScale GmbH <info@indiscale.com>
+ * Copyright (C) 2020-2021 Timm Fitschen <t.fitschen@indiscale.com>
  *
- * This program is free software: you can redistribute it and/or modify it under the terms of the
- * GNU Affero General Public License as published by the Free Software Foundation, either version 3
- * of the License, or (at your option) any later version.
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
  *
- * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without
- * even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Affero General Public License for more details.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
  *
- * You should have received a copy of the GNU Affero General Public License along with this program.
- * If not, see <https://www.gnu.org/licenses/>.
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <https://www.gnu.org/licenses/>.
  *
  * ** end header
  */
@@ -24,7 +28,8 @@ import java.util.LinkedList;
 import java.util.List;
 import org.caosdb.server.database.BackendTransaction;
 import org.caosdb.server.database.exceptions.EntityDoesNotExistException;
-import org.caosdb.server.datatype.ReferenceDatatype;
+import org.caosdb.server.datatype.CollectionValue;
+import org.caosdb.server.datatype.IndexedSingleValue;
 import org.caosdb.server.datatype.ReferenceValue;
 import org.caosdb.server.entity.EntityInterface;
 import org.caosdb.server.entity.Message;
@@ -113,6 +118,53 @@ public class RetrieveFullEntity extends BackendTransaction {
     }
   }
 
+  /**
+   * Recursively resolve the reference values of the list of reference property `p` (but only the
+   * selected sub-properties).
+   */
+  private void resolveReferenceListProperty(
+      Property p, List<Selection> selections, String propertyName) {
+    try {
+      p.parseValue();
+    } catch (Message m) {
+      p.addError(m);
+    }
+
+    CollectionValue values = (CollectionValue) p.getValue();
+    for (IndexedSingleValue sv : values) {
+      resolveReferenceValue((ReferenceValue) sv.getWrapped(), selections, propertyName);
+    }
+  }
+
+  /**
+   * Recursively resolve the reference values of the reference property `p` (but only the selected
+   * sub-properties).
+   */
+  private void resolveReferenceProperty(
+      Property p, List<Selection> selections, String propertyName) {
+    try {
+      p.parseValue();
+    } catch (Message m) {
+      p.addError(m);
+    }
+
+    resolveReferenceValue((ReferenceValue) p.getValue(), selections, propertyName);
+  }
+
+  /**
+   * Recursively resolve the reference value.
+   *
+   * <p>To be called by {@link #resolveReferenceListProperty(Property, List, String)} and {@link
+   * #resolveReferenceProperty(Property, List, String)}.
+   */
+  private void resolveReferenceValue(
+      ReferenceValue value, List<Selection> selections, String propertyName) {
+    RetrieveEntity ref = new RetrieveEntity(value.getId());
+    // recursion! (Only for the matching selections)
+    retrieveFullEntity(ref, getSubSelects(selections, propertyName));
+    value.setEntity(ref, true);
+  }
+
   /**
    * Retrieve the Entities which match the selections and are referenced by the Entity 'e'.
    *
@@ -122,55 +174,47 @@ public class RetrieveFullEntity extends BackendTransaction {
   public void retrieveSubEntities(EntityInterface e, List<Selection> selections) {
     for (final Selection s : selections) {
       String propertyName = s.getSelector();
-      if (s.getSubselection() != null) {
-
-        // Find matching (i.e. referencing) Properties
-        for (Property p : e.getProperties()) {
-          // get reference properties by name.
-          if (propertyName.equalsIgnoreCase(p.getName())
-              && p.getDatatype() instanceof ReferenceDatatype) {
-            if (p.getValue() != null) {
-              try {
-                p.parseValue();
-              } catch (Message m) {
-                p.addError(m);
+      for (Property p : e.getProperties()) {
+        if (s.getSubselection() != null) {
+          // The presence of sub-selections means that the properties are
+          // expected to be references (list or plain).
+          if (p.getValue() != null) {
+            if (propertyName.equalsIgnoreCase(p.getName())) {
+              if (p.isReference()) {
+                // handle (single) reference properties with matching name...
+                resolveReferenceProperty(p, selections, propertyName);
+                continue;
+              } else if (p.isReferenceList()) {
+                // handle (list) reference properties with matching name...
+                resolveReferenceListProperty(p, selections, propertyName);
+                continue;
               }
-
-              ReferenceValue value = (ReferenceValue) p.getValue();
-              RetrieveEntity ref = new RetrieveEntity(value.getId());
-              // recursion! (Only for the matching selections)
-              retrieveFullEntity(ref, getSubSelects(selections, propertyName));
-              value.setEntity(ref, true);
-            }
-            continue;
-          }
-          try {
-            boolean isSubtype = execute(new IsSubType(p.getId(), propertyName)).isSubType();
-            if (isSubtype) {
-              if (p.getValue() != null) {
-                if (p.getDatatype() instanceof ReferenceDatatype) {
-                  try {
-                    p.parseValue();
-                  } catch (Message m) {
-                    p.addError(m);
+            } else {
+              try {
+                boolean isSubtype = execute(new IsSubType(p.getId(), propertyName)).isSubType();
+                if (isSubtype) {
+                  // ... handle reference properties that are a subtype of `propertyName`.
+                  if (p.getValue() != null) {
+                    if (p.isReference()) {
+                      resolveReferenceProperty(p, selections, propertyName);
+                    } else if (p.isReferenceList()) {
+                      resolveReferenceListProperty(p, selections, propertyName);
+                    }
                   }
 
-                  ReferenceValue value = (ReferenceValue) p.getValue();
-                  RetrieveEntity ref = new RetrieveEntity(value.getId());
-                  // recursion! (Only for the matching selections)
-                  retrieveFullEntity(ref, getSubSelects(selections, propertyName));
-                  value.setEntity(ref, true);
+                  // the name is set the the super-types name! Otherwise, clients
+                  // wouldn't know what this property is supposed to be.
                   p.setName(propertyName);
                 }
+              } catch (EntityDoesNotExistException exc) {
+                // unknown parent name.
               }
             }
-          } catch (EntityDoesNotExistException exc) {
-            // unknown parent name.
           }
-        }
-      } else {
-        for (Property p : e.getProperties()) {
+        } else {
+          // no subselections - no need to resolve any references...
           if (!propertyName.equalsIgnoreCase(p.getName())) {
+            // ... we only need to cover property sub-typing
             try {
               boolean isSubtype = execute(new IsSubType(p.getId(), propertyName)).isSubType();
               if (isSubtype) {

src/main/java/org/caosdb/server/entity/Entity.java

@@ -41,6 +41,7 @@ import org.caosdb.server.datatype.AbstractCollectionDatatype;
 import org.caosdb.server.datatype.AbstractDatatype;
 import org.caosdb.server.datatype.CollectionValue;
 import org.caosdb.server.datatype.GenericValue;
+import org.caosdb.server.datatype.ReferenceDatatype;
 import org.caosdb.server.datatype.Value;
 import org.caosdb.server.entity.Message.MessageType;
 import org.caosdb.server.entity.container.ParentContainer;
@@ -1159,4 +1160,16 @@ public class Entity extends AbstractObservable implements EntityInterface {
     }
     return getId().toString();
   }
+
+  @Override
+  public boolean isReference() {
+    return this.hasDatatype() && this.getDatatype() instanceof ReferenceDatatype;
+  }
+
+  @Override
+  public boolean isReferenceList() {
+    return this.hasDatatype()
+        && this.getDatatype() instanceof AbstractCollectionDatatype
+        && ((AbstractCollectionDatatype) getDatatype()).getDatatype() instanceof ReferenceDatatype;
+  }
 }

src/main/java/org/caosdb/server/entity/EntityInterface.java

@@ -194,4 +194,13 @@ public interface EntityInterface
   public abstract void setVersion(Version version);
 
   public abstract void addToElement(Element element, SetFieldStrategy strategy);
+
+  /** Return true iff the data type is present and is an instance of ReferenceDatatype. */
+  public abstract boolean isReference();
+
+  /**
+   * Return true iff the data type is present, an instance of AbstractCollectionDatatype and the
+   * AbstractCollectionDatatype's elements' data type is an instance of ReferenceDatatype.
+   */
+  public abstract boolean isReferenceList();
 }

src/main/java/org/caosdb/server/entity/wrapper/EntityWrapper.java

@@ -580,4 +580,14 @@ public class EntityWrapper implements EntityInterface {
   public void addToElement(Element element, SetFieldStrategy strategy) {
     this.entity.addToElement(element, strategy);
   }
+
+  @Override
+  public boolean isReference() {
+    return this.entity.isReference();
+  }
+
+  @Override
+  public boolean isReferenceList() {
+    return this.entity.isReferenceList();
+  }
 }

src/main/java/org/caosdb/server/entity/xml/EntityToElementStrategy.java

@@ -4,8 +4,8 @@
  *
  * Copyright (C) 2018 Research Group Biomedical Physics,
  * Max-Planck-Institute for Dynamics and Self-Organization Göttingen
- * Copyright (C) 2020 Timm Fitschen <t.fitschen@indiscale.com>
- * Copyright (C) 2020 IndiScale GmbH <info@indiscale.com>
+ * Copyright (C) 2020-2021 Timm Fitschen <t.fitschen@indiscale.com>
+ * Copyright (C) 2020-2021 IndiScale GmbH <info@indiscale.com>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU Affero General Public License as
@@ -25,6 +25,8 @@
 package org.caosdb.server.entity.xml;
 
 import org.apache.shiro.SecurityUtils;
+import org.caosdb.server.datatype.CollectionValue;
+import org.caosdb.server.datatype.IndexedSingleValue;
 import org.caosdb.server.datatype.ReferenceValue;
 import org.caosdb.server.entity.EntityInterface;
 import org.caosdb.server.entity.Message;
@@ -137,8 +139,7 @@ public class EntityToElementStrategy implements ToElementStrategy {
         // CheckValueParsable job.
       }
 
-      if (entity.getValue() instanceof ReferenceValue
-          && setFieldStrategy.isToBeSet("_referenced")) {
+      if (entity.isReference() && setFieldStrategy.isToBeSet("_referenced")) {
         // Append the complete entity. This needs to be done when we are
         // processing SELECT Queries.
         EntityInterface ref = ((ReferenceValue) entity.getValue()).getEntity();
@@ -151,6 +152,26 @@ public class EntityToElementStrategy implements ToElementStrategy {
           // adding the reference id as well.
           return;
         }
+      } else if (entity.isReferenceList() && setFieldStrategy.isToBeSet("_referenced")) {
+        // Append the all referenced entities. This needs to be done when we are
+        // processing SELECT Queries.
+        boolean skipValue = false;
+        for (IndexedSingleValue sv : ((CollectionValue) entity.getValue())) {
+          EntityInterface ref = ((ReferenceValue) sv.getWrapped()).getEntity();
+          if (ref != null) {
+            if (entity.hasDatatype()) {
+              setDatatype(entity, element);
+            }
+            Element valueElem = new Element("Value");
+            ref.addToElement(valueElem, setFieldStrategy);
+            element.addContent(valueElem);
+            skipValue = true;
+          }
+        }
+        if (skipValue)
+          // the referenced entity has been appended. Return here to suppress
+          // adding the reference id as well.
+          return;
       }
 
       if (setFieldStrategy.isToBeSet("value")) {

src/main/java/org/caosdb/server/jobs/core/CheckRefidValid.java

@@ -4,8 +4,8 @@
  *
  * Copyright (C) 2018 Research Group Biomedical Physics,
  *   Max-Planck-Institute for Dynamics and Self-Organization Göttingen
- * Copyright (C) 2020 IndiScale GmbH <info@indiscale.com>
- * Copyright (C) 2020 Timm Fitschen <t.fitschen@indiscale.com>
+ * Copyright (C) 2020-2021 IndiScale GmbH <info@indiscale.com>
+ * Copyright (C) 2020-2021 Timm Fitschen <t.fitschen@indiscale.com>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU Affero General Public License as
@@ -26,10 +26,8 @@ package org.caosdb.server.jobs.core;
 
 import org.caosdb.server.database.exceptions.EntityDoesNotExistException;
 import org.caosdb.server.database.exceptions.EntityWasNotUniqueException;
-import org.caosdb.server.datatype.AbstractCollectionDatatype;
 import org.caosdb.server.datatype.CollectionValue;
 import org.caosdb.server.datatype.IndexedSingleValue;
-import org.caosdb.server.datatype.ReferenceDatatype;
 import org.caosdb.server.datatype.ReferenceValue;
 import org.caosdb.server.entity.Entity;
 import org.caosdb.server.entity.EntityInterface;
@@ -49,7 +47,7 @@ public class CheckRefidValid extends EntityJob {
   @Override
   public final void run() {
     try {
-      if (isReference(getEntity())) {
+      if (assureReference(getEntity())) {
         if (getEntity().hasValue()) {
 
           // parse referenced id
@@ -58,11 +56,9 @@ public class CheckRefidValid extends EntityJob {
             return;
           }
 
-          if (getEntity().getDatatype() instanceof ReferenceDatatype) {
+          if (getEntity().isReference()) {
             checkRefValue((ReferenceValue) getEntity().getValue());
-          } else if (getEntity().getDatatype() instanceof AbstractCollectionDatatype
-              && ((AbstractCollectionDatatype) getEntity().getDatatype()).getDatatype()
-                  instanceof ReferenceDatatype) {
+          } else if (getEntity().isReferenceList()) {
             final CollectionValue vals = (CollectionValue) getEntity().getValue();
             for (final IndexedSingleValue v : vals) {
               if (v != null && v.getWrapped() != null) {
@@ -175,14 +171,15 @@ public class CheckRefidValid extends EntityJob {
     return true;
   }
 
-  private final boolean isReference(final EntityInterface entity) {
+  /**
+   * Return true if this is a reference or a list of reference property.
+   *
+   * <p>If the data type is not present (yet), append a data type listener which calls this job
+   * again when the data type is present.
+   */
+  private final boolean assureReference(final EntityInterface entity) {
     if (entity.hasDatatype()) {
-      if (entity.getDatatype() instanceof ReferenceDatatype) {
-        return true;
-      } else if (entity.getDatatype() instanceof AbstractCollectionDatatype) {
-        return ((AbstractCollectionDatatype) entity.getDatatype()).getDatatype()
-            instanceof ReferenceDatatype;
-      }
+      return entity.isReference() || entity.isReferenceList();
     } else {
       entity.acceptObserver(this);
     }

src/test/java/org/caosdb/server/entity/EntityTest.java

+/*
+ * ** header v3.0
+ * This file is a part of the CaosDB Project.
+ *
+ * Copyright (C) 2021 IndiScale GmbH <info@indiscale.com>
+ * Copyright (C) 2021 Timm Fitschen <t.fitschen@indiscale.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <https://www.gnu.org/licenses/>.
+ *
+ * ** end header
+ */
+package org.caosdb.server.entity;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.caosdb.server.datatype.IntegerDatatype;
+import org.junit.Test;
+
+public class EntityTest {
+
+  @Test
+  public void testIsReference() {
+    EntityInterface entity = new RetrieveEntity("test");
+    assertFalse(entity.isReference());
+
+    entity.setDatatype(new IntegerDatatype());
+    assertFalse(entity.isReference());
+
+    entity.setDatatype("Person");
+    assertTrue(entity.isReference());
+  }
+
+  @Test
+  public void testIsReferenceList() {
+    EntityInterface entity = new RetrieveEntity("test");
+    assertFalse(entity.isReferenceList());
+
+    entity.setDatatype(new IntegerDatatype());
+    assertFalse(entity.isReferenceList());
+
+    entity.setDatatype("Person");
+    assertFalse(entity.isReferenceList());
+
+    entity.setDatatype("LIST<Person>");
+    assertTrue(entity.isReferenceList());
+  }
+}

src/test/java/org/caosdb/server/entity/xml/PropertyToElementStrategyTest.java

@@ -77,6 +77,7 @@ public class PropertyToElementStrategyTest {
     house.addProperty(houseHeight);
     windowProperty = new Property(2345);
     windowProperty.setName("window");
+    windowProperty.setDatatype("window");
     windowProperty.setValue(new ReferenceValue(window.getId()));
     house.addProperty(windowProperty);