[thread-search] Add loadFromColumn option to Attribute

Summary:
The isSearchIndexed attribute gets bulk reset when we drop the search
index, however, the JSON values were not updated to reflect the new
column values. We don't care about notifications to this field, so in
order to make bulk clearing ok this diff adds a new loadFromColumn option to
Attributes which causes the value to be loaded from the SQL table column
rather than the JSON blob.

Test Plan:
Run locally, drop the search index, make sure we load from the
column rather than the JSON when re-indexing

Reviewers: juan, evan

Reviewed By: juan, evan

Differential Revision: https://phab.nylas.com/D3739
This commit is contained in:
Mark Hahnenberg 2017-01-18 17:47:34 -08:00
parent 85c9439f8f
commit 157501130b
5 changed files with 32 additions and 4 deletions

View file

@ -15,6 +15,9 @@ export default class AttributeBoolean extends Attribute {
fromJSON(val) {
return ((val === 'true') || (val === true)) || false;
}
fromColumn(val) {
return (val === 1) || false;
}
columnSQL() {
const defaultValue = this.defaultValue ? 1 : 0;
return `${this.jsonKey} INTEGER DEFAULT ${defaultValue}`;

View file

@ -10,11 +10,15 @@ The Attribute class also exposes convenience methods for generating {Matcher} ob
Section: Database
*/
export default class Attribute {
constructor({modelKey, queryable, jsonKey, defaultValue}) {
constructor({modelKey, queryable, jsonKey, defaultValue, loadFromColumn}) {
this.modelKey = modelKey;
this.jsonKey = jsonKey || modelKey;
this.queryable = queryable;
this.defaultValue = defaultValue;
if (loadFromColumn && !queryable) {
throw new Error('loadFromColumn requires queryable');
}
this.loadFromColumn = loadFromColumn;
}
_assertPresentAndQueryable(fnName, val) {
@ -77,4 +81,12 @@ export default class Attribute {
fromJSON(val) {
return val || null;
}
fromColumn(val) {
return this.fromJSON(val);
}
needsColumn() {
return this.queryable && this.columnSQL && this.jsonKey !== 'id'
}
}

View file

@ -292,6 +292,13 @@ export default class ModelQuery {
return result.map((row) => {
const json = JSON.parse(row.data, Utils.registeredObjectReviver)
const object = (new this._klass()).fromJSON(json);
for (const attrName of Object.keys(this._klass.attributes)) {
const attr = this._klass.attributes[attrName];
if (!attr.needsColumn() || !attr.loadFromColumn) {
continue;
}
object[attr.modelKey] = attr.fromColumn(row[attr.jsonKey]);
}
for (const attr of this._includeJoinedData) {
let value = row[attr.jsonKey];
if (value === AttributeJoinedData.NullPlaceholder) {
@ -331,6 +338,13 @@ export default class ModelQuery {
result = `\`${this._klass.name}\`.\`id\``;
} else {
result = `\`${this._klass.name}\`.\`data\``;
for (const attrName of Object.keys(this._klass.attributes)) {
const attr = this._klass.attributes[attrName];
if (!attr.needsColumn() || !attr.loadFromColumn) {
continue;
}
result += `, ${attr.jsonKey} `;
}
this._includeJoinedData.forEach((attr) => {
result += `, ${attr.selectSQL(this._klass)} `;
})

View file

@ -111,6 +111,7 @@ class Thread extends ModelWithMetadata {
modelKey: 'isSearchIndexed',
jsonKey: 'is_search_indexed',
defaultValue: false,
loadFromColumn: true,
}),
})

View file

@ -42,9 +42,7 @@ export default class DatabaseSetupQueryBuilder {
// Identify attributes of this class that can be matched against. These
// attributes need their own columns in the table
const columnAttributes = attributes.filter(attr =>
attr.queryable && attr.columnSQL && attr.jsonKey !== 'id'
);
const columnAttributes = attributes.filter(attr => attr.needsColumn())
const columns = ['id TEXT PRIMARY KEY', 'data BLOB']
columnAttributes.forEach(attr => columns.push(attr.columnSQL()));