A draft refactoring for ACLs based on #110.

* Remove persistent if statements, especially checking whether aclSql is null.
* Make it clear that there are two choices, whether aclSql is null,
  and whether we are coming from getDocContent or isUserAuthorized.
* Make most of the config decisions at init-time.

Notes:
* Indentation is incorrect to reduce diffs.
* Additional refactoring can eliminate the additional arguments for
  buildAcl and getAclFromDb by moving them inside of AclHandler.

src/com/google/enterprise/adaptor/database/DatabaseAdaptor.java

@@ -125,6 +125,7 @@ private static boolean isNullOrEmptyString(String str) {
   private String user;
   private String password;
   private UniqueKey uniqueKey;
+  private AclHandler aclHandler;
   private String everyDocIdSql;
   private String singleDocContentSql;
   private MetadataColumns metadataColumns;
@@ -311,6 +312,14 @@ public void init(AdaptorContext context) throws Exception {
         .setContentSqlColumns(cfg.getValue("db.singleDocContentSqlParameters"))
         .setAclSqlColumns(cfg.getValue("db.aclSqlParameters"));
 
+    if (aclSql == null) {
+      aclHandler = new AclHandler(singleDocContentSql,
+          ukBuilder.getContentSqlColumns()).withResultSetStrategy();
+    } else {
+      aclHandler = new AclHandler(aclSql,
+          ukBuilder.getAclSqlColumns()).withQueryStrategy();
+    }
+
     // Verify all column names.
     try (Connection conn = makeNewConnection()) {
       Map<String, SqlType> columnTypes =
@@ -674,7 +683,7 @@ public void getDocContent(Request req, Response resp) throws IOException {
       // Generate response metadata first.
       addMetadataToResponse(resp, rs);
       // Generate Acl.
-      resp.setAcl(getAcl(conn, rs, id.getUniqueId()));
+      resp.setAcl(aclHandler.forContent().getAcl(conn, rs, id.getUniqueId()));
       // Generate response body.
       // In database adaptor's case, we almost never want to follow the URLs.
       // One record means one document.
@@ -689,28 +698,72 @@ public static void main(String[] args) {
     AbstractAdaptor.main(new DatabaseAdaptor(), args);
   }
 
-  private Acl getAcl(Connection conn, ResultSet contentRs, String uniqueId)
-      throws SQLException {
-    if (aclSql != null || contentRs == null) {
-      try (PreparedStatement stmt = getAclFromDb(conn, uniqueId);
+  private class AclHandler {
+    String sql;
+    List<String> sqlColumns;
+    ResultSetNext rsNext;
+    Acl defaultAcl;
+    Strategy contentStrategy;
+    Strategy authzStrategy;
+
+    AclHandler(String sql, List<String> sqlColumns) {
+      this.sql = sql;
+      this.sqlColumns = sqlColumns;
+      this.authzStrategy = new QueryStrategy();
+    }
+
+    public AclHandler withQueryStrategy() {
+      contentStrategy = new QueryStrategy();
+      rsNext = ResultSetNext.PROCESS_ALL_ROWS;
+      defaultAcl = Acl.EMPTY;
+      return this;
+    }
+
+    public AclHandler withResultSetStrategy() {
+      contentStrategy = new ResultSetStrategy();
+      rsNext = ResultSetNext.PROCESS_ONE_ROW;
+      defaultAcl = null;
+      return this;
+    }
+
+    public Strategy forContent() {
+      return contentStrategy;
+    }
+
+    public Strategy forAuthz() {
+      return authzStrategy;
+    }
+
+    public abstract class Strategy {
+      public abstract Acl getAcl(Connection conn, ResultSet rs, String uniqueId)
+          throws SQLException;
+    }
+
+    private class QueryStrategy extends Strategy {
+      public Acl getAcl(Connection conn, ResultSet ignored, String uniqueId)
+          throws SQLException {
+      try (PreparedStatement stmt = getAclFromDb(conn, uniqueId, sql, sqlColumns);
           ResultSet rs = stmt.executeQuery()) {
         log.finer("got acl");
         boolean hasResult = rs.next();
         if (!hasResult) {
           // empty Acl ensures adaptor will mark this document as secure
           return Acl.EMPTY;
-        } else if (aclSql != null) {
-          return buildAcl(rs, aclPrincipalDelimiter, aclNamespace,
-              ResultSetNext.PROCESS_ALL_ROWS, Acl.EMPTY);
         } else {
           return buildAcl(rs, aclPrincipalDelimiter, aclNamespace,
-              ResultSetNext.PROCESS_ONE_ROW, null);
+              rsNext, defaultAcl);
         }
       }
-    } else {
+      }
+    }
+
+    private class ResultSetStrategy extends Strategy {
+      public Acl getAcl(Connection conn, ResultSet contentRs, String uniqueId)
+          throws SQLException {
       // Generate ACL if it came as part of result to singleDocContentSql.
       return buildAcl(contentRs, aclPrincipalDelimiter, aclNamespace,
-          ResultSetNext.PROCESS_ONE_ROW, null);
+          rsNext, defaultAcl);
+      }
     }
   }
 
@@ -830,16 +883,10 @@ private PreparedStatement getDocFromDb(Connection conn,
   }
 
   private PreparedStatement getAclFromDb(Connection conn,
-      String uniqueId) throws SQLException {
-    PreparedStatement st;
-    // aclSql will only be null when called from isUserAuthorized.
-    if (aclSql == null) {
-      st = conn.prepareStatement(singleDocContentSql);
-      uniqueKey.setContentSqlValues(st, uniqueId);
-    } else {
-      st = conn.prepareStatement(aclSql);
-      uniqueKey.setAclSqlValues(st, uniqueId);
-    }
+      String uniqueId, String sql, List<String> sqlColumns)
+      throws SQLException {
+    PreparedStatement st = conn.prepareStatement(sql);
+    uniqueKey.setSqlValues(st, uniqueId, sqlColumns);
     log.log(Level.FINER, "about to get acl: {0}",  uniqueId);
     return st;
   }
@@ -1128,7 +1175,7 @@ public Map<DocId, AuthzStatus> isUserAuthorized(AuthnIdentity userIdentity,
       try (Connection conn = makeNewConnection()) {
         for (DocId id : ids) {
           log.log(Level.FINE, "about to get acl of doc {0}", id);
-          Acl acl = getAcl(conn, null, id.getUniqueId());
+          Acl acl = aclHandler.forAuthz().getAcl(conn, null, id.getUniqueId());
           if (acl == null) {
             result.put(id, AuthzStatus.PERMIT);
           } else if (userIdentity == null || userIdentity.getUser() == null) {

src/com/google/enterprise/adaptor/database/UniqueKey.java

@@ -112,7 +112,7 @@ String makeUniqueId(ResultSet rs) throws SQLException, URISyntaxException {
     return sb.toString().substring(1);
   }
 
-  private void setSqlValues(PreparedStatement st, String uniqueId,
+  void setSqlValues(PreparedStatement st, String uniqueId,
       List<String> sqlCols) throws SQLException {
     // parse on / that isn't preceded by escape char _
     // (a / that is preceded by _ is part of column value)