Skip to content
This repository was archived by the owner on Jun 23, 2022. It is now read-only.

First support for secondary metadata query. #114

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

First support for secondary metadata query. #114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b99a471
First support for secondary metadata query.
aptls Dec 16, 2017
567df68
Code review changes.
Dec 19, 2017
40f13df
Code review changes.
Jan 4, 2018
6696f19
Code review changes.
Jan 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 113 additions & 12 deletions src/com/google/enterprise/adaptor/database/DatabaseAdaptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ private static boolean isNullOrEmptyString(String str) {
private String everyDocIdSql;
private String singleDocContentSql;
private MetadataColumns metadataColumns;
private String extraMetadataSql;
private MetadataColumns extraMetadataColumns;
private ResponseGenerator respGenerator;
private String aclSql;
private String aclPrincipalDelimiter;
Expand All @@ -150,6 +152,9 @@ public void initConfig(Config config) {
// when set to true, if "db.metadataColumns" is blank, it will use all
// returned columns as metadata.
config.addKey("db.includeAllColumnsAsMetadata", "false");
config.addKey("db.extraMetadataSql", "");
config.addKey("db.extraMetadataSqlParameters", "");
config.addKey("db.extraMetadataColumns", "");
config.addKey("db.modeOfOperation", null);
config.addKey("db.updateSql", "");
config.addKey("db.aclSql", "");
Expand Down Expand Up @@ -234,6 +239,27 @@ public void init(AdaptorContext context) throws Exception {
}
}

if (!cfg.getValue("db.extraMetadataSql").isEmpty()) {
extraMetadataSql = cfg.getValue("db.extraMetadataSql");
log.config("extra metadata sql: " + extraMetadataSql);
String extraMetadataColumnsConfig =
cfg.getValue("db.extraMetadataColumns");
if (extraMetadataColumnsConfig.isEmpty()) {
extraMetadataColumns = null;
log.config("extra metadata columns: Use all columns in ResultSet");
} else {
extraMetadataColumns = new MetadataColumns(extraMetadataColumnsConfig);
log.config("extra metadata columns: " + extraMetadataColumns);
}
} else {
if (!cfg.getValue("db.extraMetadataColumns").isEmpty()) {
ignored.add("db.extraMetadataColumns");
}
if (!cfg.getValue("db.extraMetadataSqlParameters").isEmpty()) {
ignored.add("db.extraMetadataSqlParameters");
}
}

modeOfOperation = cfg.getValue("db.modeOfOperation");
log.config("mode of operation: " + modeOfOperation);

Expand All @@ -258,6 +284,15 @@ public void init(AdaptorContext context) throws Exception {
} else if (!metadataColumns.isEmpty()) {
ignored.add("db.metadataColumns");
}
if (!cfg.getValue("db.extraMetadataSql").isEmpty()) {
ignored.add("db.extraMetadataSql");
}
if (!cfg.getValue("db.extraMetadataColumns").isEmpty()) {
ignored.add("db.extraMetadataColumns");
}
if (!cfg.getValue("db.extraMetadataSqlParameters").isEmpty()) {
ignored.add("db.extraMetadataSqlParameters");
}
}
} else if (singleDocContentSql.isEmpty()) {
throw new InvalidConfigurationException(
Expand Down Expand Up @@ -311,7 +346,9 @@ public void init(AdaptorContext context) throws Exception {
= new UniqueKey.Builder(cfg.getValue("db.uniqueKey"))
.setDocIdIsUrl(docIdIsUrl)
.setContentSqlColumns(cfg.getValue("db.singleDocContentSqlParameters"))
.setAclSqlColumns(cfg.getValue("db.aclSqlParameters"));
.setAclSqlColumns(cfg.getValue("db.aclSqlParameters"))
.setMetadataSqlColumns(
cfg.getValue("db.extraMetadataSqlParameters"));

// Verify all column names.
try (Connection conn = makeNewConnection()) {
Expand All @@ -332,6 +369,10 @@ public void init(AdaptorContext context) throws Exception {
"db.metadataColumns", metadataColumns.keySet());
}
}
if (extraMetadataColumns != null) {
verifyColumnNames(conn, "db.extraMetadataSql", extraMetadataSql,
"db.extraMetadataColumns", extraMetadataColumns.keySet());
}
if (respGenerator instanceof ResponseGenerator.SingleColumnContent) {
ResponseGenerator.SingleColumnContent content =
(ResponseGenerator.SingleColumnContent) respGenerator;
Expand Down Expand Up @@ -477,6 +518,9 @@ public void getDocIds(DocIdPusher pusher) throws IOException,
builder.setDeleteFromIndex(true);
} else if ("urlAndMetadataLister".equals(modeOfOperation)) {
addMetadataToRecordBuilder(builder, rs);
if (extraMetadataSql != null) {
addExtraMetadataToRecordBuilder(builder, conn, id.getUniqueId());
}
}
DocIdPusher.Record record = builder.build();
log.log(Level.FINEST, "doc id: {0}", id);
Expand All @@ -501,15 +545,10 @@ private interface MetadataHandler {
/*
* Adds all specified metadata columns to the record or response being built.
*/
private void addMetadata(MetadataHandler meta, ResultSet rs)
throws SQLException, IOException {
private void addMetadata(MetadataHandler meta, ResultSet rs,
MetadataColumns columns) throws SQLException, IOException {
ResultSetMetaData rsMetaData = rs.getMetaData();
synchronized (this) {
if (metadataColumns == null) {
metadataColumns = new MetadataColumns(rsMetaData);
}
}
for (Map.Entry<String, String> entry : metadataColumns.entrySet()) {
for (Map.Entry<String, String> entry : columns.entrySet()) {
int index;
try {
index = rs.findColumn(entry.getKey());
Expand Down Expand Up @@ -633,25 +672,73 @@ private void addMetadata(MetadataHandler meta, ResultSet rs)
@VisibleForTesting
void addMetadataToRecordBuilder(final DocIdPusher.Record.Builder builder,
ResultSet rs) throws SQLException, IOException {
synchronized (this) {
if (metadataColumns == null) {
metadataColumns = new MetadataColumns(rs.getMetaData());
}
}
addMetadata(
new MetadataHandler() {
@Override public void addMetadata(String k, String v) {
builder.addMetadata(k, v);
}
},
rs);
rs, metadataColumns);
}

@VisibleForTesting
void addMetadataToResponse(final Response resp, ResultSet rs)
throws SQLException, IOException {
synchronized (this) {
if (metadataColumns == null) {
metadataColumns = new MetadataColumns(rs.getMetaData());
Copy link
Contributor

@wiarlawd wiarlawd Dec 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line isn't tested, since you lifted this out of the helper in two places, only the other line is executed.

}
}
addMetadata(
new MetadataHandler() {
@Override public void addMetadata(String k, String v) {
resp.addMetadata(k, v);
}
},
rs);
rs, metadataColumns);
}

private void addExtraMetadataToRecordBuilder(
final DocIdPusher.Record.Builder builder, Connection conn,
String uniqueId) throws SQLException, IOException {
addExtraMetadata(conn, uniqueId,
new MetadataHandler() {
@Override public void addMetadata(String k, String v) {
builder.addMetadata(k, v);
}
});
}

private void addExtraMetadataToResponse(final Response resp, Connection conn,
String uniqueId) throws SQLException, IOException {
addExtraMetadata(conn, uniqueId,
new MetadataHandler() {
@Override public void addMetadata(String k, String v) {
resp.addMetadata(k, v);
}
});
}

private void addExtraMetadata(Connection conn, String uniqueId,
MetadataHandler handler) throws SQLException, IOException {
try (PreparedStatement stmt = prepareExtraMetadataStatement(conn, uniqueId);
ResultSet rs = stmt.executeQuery()) {
synchronized (this) {
if (extraMetadataColumns == null) {
extraMetadataColumns = new MetadataColumns(rs.getMetaData());
}
}
while (rs.next()) {
addMetadata(handler, rs, extraMetadataColumns);
}
} catch (SQLException ex) {
throw new IOException(ex);
Copy link
Contributor

@wiarlawd wiarlawd Dec 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is untested. I'm not sure how to trigger this. A flat invalid query would fail in validation. Maybe a type mismatch in the parameter expression: <int-field> = ? with a character parameter? I can look harder if you can't find something easily.

}
}

/** Gives the bytes of a document referenced with id. */
Expand All @@ -675,6 +762,9 @@ public void getDocContent(Request req, Response resp) throws IOException {
}
// Generate response metadata first.
addMetadataToResponse(resp, rs);
if (extraMetadataSql != null) {
addExtraMetadataToResponse(resp, conn, id.getUniqueId());
}
// Generate Acl if aclSql is provided.
if (aclSql != null) {
resp.setAcl(getAcl(conn, id.getUniqueId()));
Expand Down Expand Up @@ -838,6 +928,14 @@ private PreparedStatement getStreamFromDb(Connection conn,
return st;
}

private PreparedStatement prepareExtraMetadataStatement(Connection conn,
String uniqueId) throws SQLException {
PreparedStatement st = conn.prepareStatement(extraMetadataSql);
uniqueKey.setMetadataSqlValues(st, uniqueId);
log.log(Level.FINER, "about to get extra metadata: {0}", uniqueId);
return st;
}

/**
* Mechanism that accepts stream of DocIdPusher.Record instances, buffers
* them, and sends them when it has accumulated maximum allowed amount per
Expand Down Expand Up @@ -921,7 +1019,7 @@ static ResponseGenerator loadResponseGenerator(Config config) {
throw new InvalidConfigurationException(errmsg, ex);
}
}

@VisibleForTesting
static ResponseGenerator loadResponseGeneratorInternal(Method method,
Map<String, String> config) {
Expand Down Expand Up @@ -1026,6 +1124,9 @@ public void getModifiedDocIds(DocIdPusher pusher)
builder.setDeleteFromIndex(true);
} else if ("urlAndMetadataLister".equals(modeOfOperation)) {
addMetadataToRecordBuilder(builder, rs);
if (extraMetadataSql != null) {
addExtraMetadataToRecordBuilder(builder, conn, id.getUniqueId());
}
}
DocIdPusher.Record record = builder.build();
log.log(Level.FINEST, "doc id: {0}", id);
Expand Down
70 changes: 56 additions & 14 deletions src/com/google/enterprise/adaptor/database/UniqueKey.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,17 @@ static enum ColumnType {
private final Map<String, ColumnType> types; // types of DocId columns
private final List<String> contentSqlCols; // columns for content query
private final List<String> aclSqlCols; // columns for acl query
private final List<String> metadataSqlCols; // columns for metadata query
private final boolean docIdIsUrl;

private UniqueKey(List<String> docIdSqlCols, Map<String, ColumnType> types,
List<String> contentSqlCols, List<String> aclSqlCols,
boolean docIdIsUrl) {
List<String> metadataSqlCols, boolean docIdIsUrl) {
this.docIdSqlCols = docIdSqlCols;
this.types = types;
this.contentSqlCols = contentSqlCols;
this.aclSqlCols = aclSqlCols;
this.metadataSqlCols = metadataSqlCols;
this.docIdIsUrl = docIdIsUrl;
}

Expand Down Expand Up @@ -114,18 +116,34 @@ String makeUniqueId(ResultSet rs) throws SQLException, URISyntaxException {

private 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)
String parts[] = uniqueId.split("(?<!_)/", -1);
if (parts.length != docIdSqlCols.size()) {
String errmsg = "Wrong number of values for primary key: "
+ "id: " + uniqueId + ", parts: " + Arrays.asList(parts);
throw new IllegalStateException(errmsg);
}
Map<String, String> zip = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
for (int i = 0; i < parts.length; i++) {
String columnValue = decodeSlashInData(parts[i]);
zip.put(docIdSqlCols.get(i), columnValue);
// Parameters to SQL queries are taken from the unique id. When
// reading extraMetadata when docIdIsUrl is true, we're now
// passing the unique id to this method for the first time. Don't
// try to split the id; we know it must be a single unique string.

// TODO(aptls): while this may be OK in that we don't want to try
// to split the id when we know it's a URL, the underlying use
// case of using value(s) from the unique id to retrieve the extra
// metadata might not want to have to use this URL as the metadata
// key. We want to change this to be able to retrieve the metadata
// key value(s) from the doc id or content result set instead of
// the key.
if (docIdIsUrl) {
zip.put(docIdSqlCols.get(0), uniqueId);
} else {
// parse on / that isn't preceded by escape char _
// (a / that is preceded by _ is part of column value)
String parts[] = uniqueId.split("(?<!_)/", -1);
if (parts.length != docIdSqlCols.size()) {
String errmsg = "Wrong number of values for primary key: "
+ "id: " + uniqueId + ", parts: " + Arrays.asList(parts);
throw new IllegalStateException(errmsg);
}
for (int i = 0; i < parts.length; i++) {
String columnValue = decodeSlashInData(parts[i]);
zip.put(docIdSqlCols.get(i), columnValue);
}
}
for (int i = 0; i < sqlCols.size(); i++) {
String colName = sqlCols.get(i);
Expand Down Expand Up @@ -171,6 +189,11 @@ void setAclSqlValues(PreparedStatement st, String uniqueId)
setSqlValues(st, uniqueId, aclSqlCols);
}

void setMetadataSqlValues(PreparedStatement st, String uniqueId)
throws SQLException {
setSqlValues(st, uniqueId, metadataSqlCols);
}

@VisibleForTesting
static String encodeSlashInData(String data) {
if (-1 == data.indexOf('/') && -1 == data.indexOf('_')) {
Expand Down Expand Up @@ -218,7 +241,7 @@ static String decodeSlashInData(String id) {
@Override
public String toString() {
return "UniqueKey(" + docIdSqlCols + "," + types + "," + contentSqlCols + ","
+ aclSqlCols + ")";
+ aclSqlCols + "," + metadataSqlCols + ")";
Copy link
Contributor

@wiarlawd wiarlawd Dec 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Har, OK. Either way is fine.

}

/** Builder to create instances of {@code UniqueKey}. */
Expand All @@ -227,6 +250,7 @@ static class Builder {
private Map<String, ColumnType> types; // types of DocId columns
private List<String> contentSqlCols; // columns for content query
private List<String> aclSqlCols; // columns for acl query
private List<String> metadataSqlCols; // columns for metadata query
private boolean docIdIsUrl = false;

/**
Expand Down Expand Up @@ -282,6 +306,7 @@ static class Builder {
docIdSqlCols = Collections.unmodifiableList(tmpNames);
contentSqlCols = docIdSqlCols;
aclSqlCols = docIdSqlCols;
metadataSqlCols = docIdSqlCols;
}

/**
Expand Down Expand Up @@ -345,6 +370,19 @@ Builder setAclSqlColumns(String aclSqlColumns)
return this;
}

Builder setMetadataSqlColumns(String metadataSqlColumns)
throws InvalidConfigurationException {
if (metadataSqlColumns == null) {
throw new NullPointerException();
}
if (!metadataSqlColumns.trim().isEmpty()) {
this.metadataSqlCols =
splitIntoNameList("db.extraMetadataSqlParameters",
metadataSqlColumns, types.keySet());
}
return this;
}

private static List<String> splitIntoNameList(String paramConfig,
String cols, Set<String> validNames) {
List<String> columnNames = new ArrayList<String>();
Expand Down Expand Up @@ -434,6 +472,10 @@ List<String> getAclSqlColumns() {
return aclSqlCols;
}

List<String> getMetadataSqlColumns() {
return metadataSqlCols;
}

/** Return the Map of column types. */
@VisibleForTesting
Map<String, ColumnType> getColumnTypes() {
Expand Down Expand Up @@ -464,7 +506,7 @@ UniqueKey build() throws InvalidConfigurationException {
+ " The key must be a single string column when docId.isUrl=true.");
}
return new UniqueKey(docIdSqlCols, types, contentSqlCols, aclSqlCols,
docIdIsUrl);
metadataSqlCols, docIdIsUrl);
}
}
}
Loading