Merge branch 'develop' into fb_sql_experiment

Author: labkey-jeckels

announcements/src/org/labkey/announcements/model/AnnouncementType.java

@@ -16,7 +16,6 @@
 package org.labkey.announcements.model;
 
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
 import org.labkey.api.announcements.CommSchema;
 import org.labkey.api.attachments.AttachmentParentType;
 import org.labkey.api.data.SQLFragment;
@@ -41,8 +40,8 @@ public static AttachmentParentType get()
     }
 
     @Override
-    public @Nullable SQLFragment getSelectParentEntityIdsSql()
+    public @NotNull SQLFragment getSelectEntityIdAndDescriptionSql()
     {
-        return new SQLFragment("SELECT EntityId FROM ").append(CommSchema.getInstance().getTableInfoAnnouncements(), "ann");
+        return new SQLFragment("SELECT EntityId, Title AS Description FROM ").append(CommSchema.getInstance().getTableInfoAnnouncements(), "ann");
     }
 }

api/src/org/labkey/api/ApiModule.java

@@ -106,7 +106,6 @@
 import org.labkey.api.module.ModuleLoader;
 import org.labkey.api.module.ModuleLoader.StartupPropertyStartupListener;
 import org.labkey.api.module.ModuleXml;
-import org.labkey.api.module.TomcatVersion;
 import org.labkey.api.query.AbstractQueryUpdateService;
 import org.labkey.api.query.AliasManager;
 import org.labkey.api.query.DetailsURL;
@@ -522,7 +521,6 @@ public void registerServlets(ServletContext servletCtx)
             Table.TestCase.class,
             TableSelectorTestCase.class,
             TempTableInClauseGenerator.TestCase.class,
-            TomcatVersion.TestCase.class,
             URLHelper.TestCase.class,
             UserManager.TestCase.class,
             ViewCategoryManager.TestCase.class,

api/src/org/labkey/api/attachments/AttachmentParentType.java

@@ -16,7 +16,7 @@
 package org.labkey.api.attachments;
 
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
+import org.labkey.api.data.CoreSchema;
 import org.labkey.api.data.SQLFragment;
 
 /**
@@ -25,7 +25,9 @@
  */
 public interface AttachmentParentType
 {
-    SQLFragment NO_ENTITY_IDS = new SQLFragment("SELECT NULL AS EntityId WHERE 1 = 0");
+    SQLFragment NO_ROWS = new SQLFragment("SELECT NULL AS EntityId, NULL AS Description WHERE 1 = 0");
+    SQLFragment PARENT_CONTAINER_SQL = new SQLFragment("SELECT EntityId, COALESCE(Name, '<Root>') AS Description FROM ")
+        .append(CoreSchema.getInstance().getTableInfoContainers());
 
     AttachmentParentType UNKNOWN = new AttachmentParentType()
     {
@@ -37,9 +39,9 @@ public String getUniqueName()
         }
 
         @Override
-        public void addWhereSql(SQLFragment sql, String parentColumn, String documentNameColumn)
+        public @NotNull SQLFragment getSelectEntityIdAndDescriptionSql()
         {
-            sql.append("0 = 1");
+            return NO_ROWS;
         }
     };
 
@@ -56,20 +58,27 @@ public void addWhereSql(SQLFragment sql, String parentColumn, String documentNam
     default void addWhereSql(SQLFragment sql, String parentColumn, String documentNameColumn)
     {
         SQLFragment selectSql = getSelectParentEntityIdsSql();
-        if (selectSql == null)
-            throw new IllegalStateException("Must override either addWhereSql() or getSelectParentEntityIdsSql()");
         sql.append(parentColumn).append(" IN (").append(selectSql).append(")");
     }
 
     /**
-     * Return a SQLFragment that selects all the EntityIds that might be attachment parents from the table(s) that
-     * provide attachments of this type, without involving the core.Documents table. For example,
-     * {@code SELECT EntityId FROM comm.Announcements}. Return null if this is not-yet-implemented or inappropriate.
-     * For example, some attachments' parents are container IDs. If the method determines that no parents exist, then
-     * return a valid query that selects no rows, for example, {@code NO_ENTITY_IDS}.
+     * Return a SQLFragment that selects just the EntityId of rows that might be attachment parents from the table(s)
+     * that provide attachments of this type, without involving the core.Documents table.
      */
-    default @Nullable SQLFragment getSelectParentEntityIdsSql()
+    default @NotNull SQLFragment getSelectParentEntityIdsSql()
     {
-        return null;
+        SQLFragment selectSql = getSelectEntityIdAndDescriptionSql();
+
+        // The returned SQL is always used inside a subselect, so the alias doesn't have to be unique
+        return new SQLFragment("SELECT EntityId FROM (").append(selectSql).append(") x");
     }
+
+    /**
+     * Return a SQLFragment that selects the EntityId and an appropriate Description of all rows that might be
+     * attachment parents from the table(s) that provide attachment parents of this type, without involving the
+     * core.Documents table. For example, {@code SELECT EntityId, Title AS Description FROM comm.Announcements}.
+     * If the method determines that no parents exist, then return a valid query that selects no rows, for example,
+     * {@code NO_ROWS}.
+     */
+    @NotNull SQLFragment getSelectEntityIdAndDescriptionSql();
 }

api/src/org/labkey/api/attachments/LookAndFeelResourceType.java

@@ -49,4 +49,10 @@ public void addWhereSql(SQLFragment sql, String parentColumn, String documentNam
         sql.append(documentNameColumn).append(" LIKE '" + AttachmentCache.LOGO_FILE_NAME_PREFIX + "%' OR ");
         sql.append(documentNameColumn).append(" LIKE '" + AttachmentCache.MOBILE_LOGO_FILE_NAME_PREFIX + "%')");
     }
+
+    @Override
+    public @NotNull SQLFragment getSelectEntityIdAndDescriptionSql()
+    {
+        return PARENT_CONTAINER_SQL;
+    }
 }

api/src/org/labkey/api/collections/LabKeyCollectors.java

@@ -6,6 +6,7 @@
 import org.json.JSONArray;
 import org.junit.Assert;
 import org.junit.Test;
+import org.labkey.api.data.SQLFragment;
 import org.labkey.api.util.HtmlString;
 import org.labkey.api.util.HtmlStringBuilder;
 
@@ -14,6 +15,7 @@
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -222,6 +224,18 @@ public static Collector<HtmlString, HtmlStringBuilder, HtmlString> joining(HtmlS
         );
     }
 
+    /**
+     * Returns a {@link Collector} that joins {@link SQLFragment}s into a single {@link SQLFragment} separated by delimiter
+     */
+    public static Collector<SQLFragment, List<SQLFragment>, SQLFragment> joining(SQLFragment delimiter) {
+        return Collector.of(
+            LinkedList::new,
+            List::add,
+            (list1, list2) -> {list1.addAll(list2); return list1;},
+            (list) -> SQLFragment.join(list, delimiter)
+        );
+    }
+
     public static class TestCase extends Assert
     {
         @Test

api/src/org/labkey/api/data/ConvertHelper.java

@@ -46,6 +46,7 @@
 import org.junit.Assert;
 import org.junit.Test;
 import org.labkey.api.collections.ConcurrentHashSet;
+import org.labkey.api.exp.PropertyType;
 import org.labkey.api.gwt.client.DefaultScaleType;
 import org.labkey.api.gwt.client.FacetingBehaviorType;
 import org.labkey.api.query.FieldKey;
@@ -1210,6 +1211,61 @@ public void testInvalidInfDoubleConverter()
                 }
             }
         }
+
+        @Test
+        public void testEmpty()
+        {
+            assertNull(JdbcType.CHAR.convert(""));
+            assertNull(JdbcType.VARCHAR.convert(""));
+            assertNull(JdbcType.LONGVARCHAR.convert(""));
+
+            // PropertyType is used for domain defined tables.
+            // I would expect these to return null.
+            assertEquals("", PropertyType.STRING.convert(""));
+            assertEquals("", PropertyType.MULTI_LINE.convert(""));
+            assertEquals("", PropertyType.XML_TEXT.convert(""));
+
+            // Since we often convert "through" string, I'm not sure this low-level
+            // method should modify "".  This could potentially mess up
+            // converting array->string->array for instance.
+            // [] -> "" -> null
+            // vs
+            // [] -> "" -> []
+            assertNull(ConvertHelper.convert("", String.class));
+            assertNull(ConvertUtils.convert(""));
+            assertNull(ConvertUtils.convert("", String.class));
+        }
+
+        @Test
+        public void testBlank()
+        {
+            // blank strings do not get the empty string special handling.
+            assertEquals(" ", JdbcType.CHAR.convert(" "));
+            assertEquals(" ", JdbcType.VARCHAR.convert(" "));
+            assertEquals(" ", JdbcType.LONGVARCHAR.convert(" "));
+            assertEquals(" ", PropertyType.STRING.convert(" "));
+            assertEquals(" ", PropertyType.MULTI_LINE.convert(" "));
+            assertEquals(" ", PropertyType.XML_TEXT.convert(" "));
+            assertEquals(" ", ConvertHelper.convert(" ", String.class));
+            assertEquals(" ", ConvertUtils.convert(" "));
+            assertEquals(" ", ConvertUtils.convert(" ", String.class));
+        }
+
+        @Test
+        public void testTrim()
+        {
+            // convert() does not trim.  That is handled in DataIterator.
+            // e.g. see SimpleTranslator.createConvertColumn()
+            assertEquals(" x ", JdbcType.CHAR.convert(" x "));
+            assertEquals(" x ", JdbcType.VARCHAR.convert(" x "));
+            assertEquals(" x ", JdbcType.LONGVARCHAR.convert(" x "));
+            assertEquals(" x ", PropertyType.STRING.convert(" x "));
+            assertEquals(" x ", PropertyType.MULTI_LINE.convert(" x "));
+            assertEquals(" x ", PropertyType.XML_TEXT.convert(" x "));
+            assertEquals(" x ", ConvertHelper.convert(" x ", String.class));
+            assertEquals(" x ", ConvertUtils.convert(" x "));
+            assertEquals(" x ", ConvertUtils.convert(" x ", String.class));
+        }
     }
 
     // Note: Keep in sync with LabKeySiteWrapper.getConversionErrorMessage()

api/src/org/labkey/api/data/JdbcType.java

@@ -89,6 +89,12 @@ protected void addSqlTypes(Collection<Integer> sqlTypes)
         {
             sqlTypes.add(Types.NCHAR);
         }
+
+        @Override
+        public Object convert(Object o) throws ConversionException
+        {
+            return VARCHAR.convert(o);
+        }
     },
 
     DECIMAL(Types.DECIMAL, BigDecimal.class, null, "numberfield")

api/src/org/labkey/api/data/SQLFragment.java

@@ -47,7 +47,6 @@
 import java.util.TreeSet;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
-import java.util.stream.StreamSupport;
 
 import static org.labkey.api.query.ExprColumn.STR_TABLE_ALIAS;
 
@@ -1339,25 +1338,23 @@ public int hashCode()
      * concatenation using the provided separator. The parameters are combined to form the new parameter list.
      *
      * @param fragments SQLFragments to join together
-     * @param separator Separator to use on the SQL portion
+     * @param separator Separator to use
      * @return A new SQLFragment that joins all the SQLFragments
      */
-    public static SQLFragment join(Iterable<SQLFragment> fragments, String separator)
+    public static SQLFragment join(Iterable<SQLFragment> fragments, SQLFragment separator)
     {
-        if (separator.contains("?"))
-            throw new IllegalStateException("separator must not include a parameter marker");
+        SQLFragment join = new SQLFragment();
+        boolean first = true;
 
-        // Join all the SQL statements
-        String sql = StreamSupport.stream(fragments.spliterator(), false)
-            .map(SQLFragment::getSQL)
-            .collect(Collectors.joining(separator));
-
-        // Collect all the parameters to a single list
-        List<?> params = StreamSupport.stream(fragments.spliterator(), false)
-            .map(SQLFragment::getParams)
-            .flatMap(Collection::stream)
-            .collect(Collectors.toList());
+        for (SQLFragment fragment : fragments)
+        {
+            if (first)
+                first = false;
+            else
+                join.append(separator);
+            join.append(fragment);
+        }
 
-        return new SQLFragment(sql, params);
+        return join;
     }
 }