ESB-1223 Fixed common search returning multiple values in case of join

.gitignore

@@ -8,3 +8,6 @@ target
 work
 *.tgz
 derby.log
+/.REVIEW/
+/.TESTING/
+/.PLAN/

cms-plugin/src/main/java/com/agiletec/plugins/jacms/aps/system/services/content/AbstractContentSearcherDAO.java

@@ -14,6 +14,7 @@
 package com.agiletec.plugins.jacms.aps.system.services.content;
 
 import com.agiletec.aps.system.SystemConstants;
+import com.agiletec.aps.system.common.SearchableFields;
 import com.agiletec.aps.system.common.entity.AbstractEntitySearcherDAO;
 import com.agiletec.aps.system.common.entity.model.ApsEntityRecord;
 import com.agiletec.aps.system.common.entity.model.EntitySearchFilter;
@@ -40,6 +41,31 @@
 public abstract class AbstractContentSearcherDAO extends AbstractEntitySearcherDAO implements IContentSearcherDAO {
 
 	private static final EntLogger _logger = EntLogFactory.getSanitizedLogger(AbstractContentSearcherDAO.class);
+
+	private static final String CONTENTID = "contentid";
+	private static final String CONTENTTYPE = "contenttype";
+
+	/**
+	 * The search keys this searcher accepts. Most name their column directly; the entity keys and the
+	 * two whose column is spelled differently are declared as aliases. <code>group</code> is accepted
+	 * because the chain this replaced accepted it - it is a code literal, not caller input, and keeping
+	 * it makes this an exact translation of that chain rather than a silent correction to it.
+	 */
+	private static final SearchableFields SEARCHABLE_FIELDS = SearchableFields.columns(
+			"descr",
+			"status",
+			"created",
+			"published",
+			"maingroup",
+			"currentversion",
+			"firsteditor",
+			"lasteditor",
+			"restriction",
+			"group")
+			.alias(IContentManager.ENTITY_ID_FILTER_KEY, CONTENTID)
+			.alias(IContentManager.ENTITY_TYPE_CODE_FILTER_KEY, CONTENTTYPE)
+			.alias(IContentManager.CONTENT_MODIFY_DATE_FILTER_KEY, "lastmodified")
+			.alias(IContentManager.CONTENT_ONLINE_FILTER_KEY, "onlinexml");
 
     @Override
     public int countContents(String[] categories, boolean orClauseCategoryFilter, 
@@ -105,36 +131,8 @@ public List<String> loadContentsId(String[] categories,
 	}
 
 	@Override
-	protected String getTableFieldName(String metadataFieldKey) {
-		if (metadataFieldKey.equals(IContentManager.ENTITY_ID_FILTER_KEY)) {
-			return this.getEntityMasterTableIdFieldName();
-		} else if (metadataFieldKey.equals(IContentManager.ENTITY_TYPE_CODE_FILTER_KEY)) {
-			return this.getEntityMasterTableIdTypeFieldName();
-		} else if (metadataFieldKey.equals(IContentManager.CONTENT_DESCR_FILTER_KEY)) {
-			return "descr";
-		} else if (metadataFieldKey.equals(IContentManager.CONTENT_STATUS_FILTER_KEY)) {
-			return "status";
-		} else if (metadataFieldKey.equals(IContentManager.CONTENT_CREATION_DATE_FILTER_KEY)) {
-			return "created";
-		} else if (metadataFieldKey.equals(IContentManager.CONTENT_MODIFY_DATE_FILTER_KEY)) {
-			return "lastmodified";
-		} else if (metadataFieldKey.equals(IContentManager.CONTENT_PUBLISH_DATE_FILTER_KEY)) {
-			return "published";
-		} else if (metadataFieldKey.equals(IContentManager.CONTENT_ONLINE_FILTER_KEY)) {
-			return "onlinexml";
-		} else if (metadataFieldKey.equals(IContentManager.CONTENT_MAIN_GROUP_FILTER_KEY)) {
-			return "maingroup";
-		} else if (metadataFieldKey.equals(IContentManager.CONTENT_CURRENT_VERSION_FILTER_KEY)) {
-			return "currentversion";
-		} else if (metadataFieldKey.equals(IContentManager.CONTENT_FIRST_EDITOR_FILTER_KEY)) {
-			return "firsteditor";
-		} else if (metadataFieldKey.equals(IContentManager.CONTENT_LAST_EDITOR_FILTER_KEY)) {
-			return "lasteditor";
-		} else if (metadataFieldKey.equals(IContentManager.CONTENT_RESTRICTION_FILTER_KEY)) {
-			return "restriction";
-		}else if (metadataFieldKey.equals(IContentManager.CONTENT_GROUP_FILTER_KEY)) {
-			return "group";
-		} else throw new RuntimeException("Chiave di ricerca '" + metadataFieldKey + "' non riconosciuta");
+	protected SearchableFields getSearchableFields() {
+		return SEARCHABLE_FIELDS;
 	}
 
 	protected PreparedStatement buildStatement(EntitySearchFilter[] filters,
@@ -156,7 +154,7 @@ protected PreparedStatement buildStatement(EntitySearchFilter[] filters,
 		//System.out.println("QUERY : " + query);
 		PreparedStatement stat = null;
 		try {
-			stat = conn.prepareStatement(query);
+			stat = this.prepareStatement(conn, query);
 			int index = 0;
 			index = super.addAttributeFilterStatementBlock(filters, index, stat);
 			index = this.addMetadataFieldFilterStatementBlock(filters, index, stat);
@@ -217,12 +215,12 @@ protected String createQueryString(EntitySearchFilter[] filters, String[] groups
 			hasAppendWhereClause = this.verifyWhereClauseAppend(query, hasAppendWhereClause);
 			this.addGroupsQueryBlock(query, groups);
 		}
+		boolean grouped = this.appendGroupByQueryBlock(filters, query, selectAll);
         if (!isCount) {
-            boolean ordered = this.appendOrderQueryBlocks(filters, query, false);
+            this.appendOrderQueryBlocks(filters, query, false, grouped);
             this.appendLimitQueryBlock(filters, query);
-        }
-		//System.out.println("********** " + query.toString());
-		return query.toString();
+		}
+		return this.toQueryString(query, isCount);
 	}
 
 	protected void addGroupsQueryBlock(StringBuffer query, Collection<String> userGroupCodes) {
@@ -282,8 +280,8 @@ protected Collection<String> getGroupsForSelect(Collection<String> userGroupCode
 	@Override
 	protected ApsEntityRecord createRecord(ResultSet result) throws Throwable {
 		ContentRecordVO contentVo = new ContentRecordVO();
-		contentVo.setId(result.getString("contentid"));
-		contentVo.setTypeCode(result.getString("contenttype"));
+		contentVo.setId(result.getString(CONTENTID));
+		contentVo.setTypeCode(result.getString(CONTENTTYPE));
 		contentVo.setDescription(result.getString("descr"));
 		contentVo.setStatus(result.getString("status"));
 		String xmlWork = result.getString("workxml");
@@ -309,11 +307,11 @@ protected String getEntityMasterTableName() {
 	}
 	@Override
 	protected String getEntityMasterTableIdFieldName() {
-		return "contentid";
+		return CONTENTID;
 	}
 	@Override
 	protected String getEntityMasterTableIdTypeFieldName() {
-		return "contenttype";
+		return CONTENTTYPE;
 	}
 
 	protected abstract String getContentRelationsTableName();

cms-plugin/src/main/java/com/agiletec/plugins/jacms/aps/system/services/content/PublicContentSearcherDAO.java

@@ -41,8 +41,7 @@ public List<String> loadContentsId(String[] categories,
 		} else {
 			groupCodes.addAll(userGroupCodes);
 		}
-		EntitySearchFilter onLineFilter = new EntitySearchFilter(IContentManager.CONTENT_ONLINE_FILTER_KEY, false);
-		filters = this.addFilter(filters, onLineFilter);
+		// the online filter is added by buildStatement, which the count goes through too - see there
 		List<String> contentsId = new ArrayList<String>();
 		Connection conn = null;
 		PreparedStatement stat = null;
@@ -66,10 +65,23 @@ public List<String> loadContentsId(String[] categories,
 		return contentsId;
 	}
 
+	/**
+	 * Restrict the search to published contents.
+	 *
+	 * <p>Applied here because this is the one method both sides pass through: <code>countContents</code>
+	 * and <code>loadContentsId</code> of the base class each call it, so the count and the list are built
+	 * from the same filter set and cannot report different row sets. Adding the filter in the list method
+	 * alone - which is what this class did - made the total count drafts the list would never return.</p>
+	 *
+	 * <p>The filter carries no value, so it emits <code>contents.onlinexml IS NOT NULL</code> with no
+	 * placeholder and binds nothing: the parameter positions below are unaffected by it.</p>
+	 */
 	@Override
 	protected PreparedStatement buildStatement(EntitySearchFilter[] filters,
 			String[] categories, boolean orClauseCategoryFilter, 
 			Collection<String> userGroupCodes, boolean isCount, boolean selectAll, Connection conn) {
+		filters = this.addFilter(filters,
+				new EntitySearchFilter(IContentManager.CONTENT_ONLINE_FILTER_KEY, false));
 		ArrayList<String> groups = new ArrayList<>();
 		ArrayList<EntitySearchFilter> remainingFilters = new ArrayList<>();
 		for (EntitySearchFilter filter : filters) {
@@ -85,7 +97,7 @@ protected PreparedStatement buildStatement(EntitySearchFilter[] filters,
 		String query = this.createQueryString(filters, groupsArr, categories, orClauseCategoryFilter, groupsForSelect, isCount, selectAll);
 		PreparedStatement stat = null;
 		try {
-			stat = conn.prepareStatement(query);
+			stat = this.prepareStatement(conn, query);
 			int index = 0;
 			index = super.addAttributeFilterStatementBlock(filters, index, stat);
 			index = this.addMetadataFieldFilterStatementBlock(filters, index, stat);

cms-plugin/src/main/java/com/agiletec/plugins/jacms/aps/system/services/resource/ResourceDAO.java

@@ -15,6 +15,7 @@
 
 import com.agiletec.aps.system.common.AbstractSearcherDAO;
 import com.agiletec.aps.system.common.FieldSearchFilter;
+import com.agiletec.aps.system.common.SearchableFields;
 import com.agiletec.aps.system.services.category.Category;
 import com.agiletec.aps.system.services.category.ICategoryManager;
 import com.agiletec.plugins.jacms.aps.system.services.resource.model.ResourceInterface;
@@ -48,6 +49,20 @@
 public class ResourceDAO extends AbstractSearcherDAO implements IResourceDAO {
 
     private static final EntLogger logger = EntLogFactory.getSanitizedLogger(ResourceDAO.class);
+
+    /** The columns of <code>resources</code> a search key may name. */
+    private static final SearchableFields SEARCHABLE_FIELDS = SearchableFields.columns(
+            "resid",
+            "restype",
+            "descr",
+            "maingroup",
+            "resourcexml",
+            "masterfilename",
+            "creationdate",
+            "lastmodified",
+            "owner",
+            "folderpath",
+            "correlationcode");
 
     private ICategoryManager categoryManager;
 
@@ -361,7 +376,7 @@ private PreparedStatement buildStatement(FieldSearchFilter[] filters, List<Strin
         String query = this.createQueryString(filters, categories, isCount);
         PreparedStatement stat = null;
         try {
-            stat = conn.prepareStatement(query);
+            stat = this.prepareStatement(conn, query);
             int index = 0;
             if (null != categories && categories.size() > 0) {
                 for (String category : categories) {
@@ -377,17 +392,19 @@ private PreparedStatement buildStatement(FieldSearchFilter[] filters, List<Strin
     }
 
     private String createQueryString(FieldSearchFilter[] filters, List<String> categories, boolean isCount) {
-        StringBuffer query = this.createBaseQueryBlock(filters, false, isCount, categories);
+        StringBuffer query = this.createBaseQueryBlock(filters, false, categories);
         this.appendMetadataFieldFilterQueryBlocks(filters, query, false);
         if (!isCount) {
             super.appendOrderQueryBlocks(filters, query, false);
             this.appendLimitQueryBlock(filters, query);
         }
-        return query.toString();
+        return this.toQueryString(query, isCount);
     }
 
-    private StringBuffer createBaseQueryBlock(FieldSearchFilter[] filters, boolean selectAll, boolean isCount, List<String> categories) {
-        StringBuffer query = super.createBaseQueryBlock(filters, isCount, selectAll);
+    private StringBuffer createBaseQueryBlock(FieldSearchFilter[] filters, boolean selectAll, List<String> categories) {
+        // count and list share one body: the category joins are the only thing that can return several
+        // rows per resource, and both sides have to see the same set
+        StringBuffer query = this.createMasterSelectQueryBlock(filters, selectAll);
         if (categories != null) {
             for (int i = 0; i < categories.size(); i++) {
                 query.append(String.format(
@@ -561,6 +578,25 @@ public void updateResourceRelations(ResourceInterface resource) {
         }
     }
 
+    /**
+     * A resource holds one <code>resourcerelations</code> row per category, and nothing in the schema
+     * forbids the same pair twice, so the joined query can return the resource more than once. Both the
+     * list and the count select distinct ids; the columns the ORDER BY references have to be projected
+     * as well, since Derby and PostgreSQL reject an ORDER BY outside the select list under DISTINCT.
+     */
+    @Override
+    protected StringBuffer createMasterSelectQueryBlock(FieldSearchFilter[] filters, boolean selectAll) {
+        if (selectAll) {
+            return super.createMasterSelectQueryBlock(filters, selectAll);
+        }
+        String masterTableName = this.getMasterTableName();
+        StringBuffer query = new StringBuffer("SELECT DISTINCT ").append(masterTableName).append(".")
+                .append(this.getMasterTableIdFieldName());
+        this.appendOrderFieldsSelectBlock(filters, query);
+        query.append(" FROM ").append(masterTableName).append(" ");
+        return query;
+    }
+
     @Override
     protected String getMasterTableName() {
         return "resources";
@@ -572,8 +608,8 @@ protected String getMasterTableIdFieldName() {
     }
 
     @Override
-    protected String getTableFieldName(String metadataFieldKey) {
-        return metadataFieldKey;
+    protected SearchableFields getSearchableFields() {
+        return SEARCHABLE_FIELDS;
     }
 
 }