Skip to content

Commit

Permalink
Closes #231 Prevent non-fatal error logs; 'SQLiteLog: (1) no such col…
Browse files Browse the repository at this point in the history
…umn' when using the Query API with a non-null WHERE and includeBlanks set to true
  • Loading branch information
vestrel00 committed Jul 11, 2022
1 parent 3b89282 commit 1f13670
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 31 deletions.
4 changes: 2 additions & 2 deletions core/src/main/java/contacts/core/Fields.kt
Original file line number Diff line number Diff line change
Expand Up @@ -1169,10 +1169,10 @@ object ContactsFields : FieldSet<ContactsField>() {
@JvmField
val Options = ContactsOptionsFields()

// Do not include in fields.
// Do not include in all.
internal val DisplayNameSource = ContactsField(Contacts.DISPLAY_NAME_SOURCE)

// Do not include in fields.
// Do not include in all.
@TargetApi(Build.VERSION_CODES.LOLLIPOP)
internal val NameRawContactId = ContactsField(Contacts.NAME_RAW_CONTACT_ID)

Expand Down
27 changes: 21 additions & 6 deletions core/src/main/java/contacts/core/Query.kt
Original file line number Diff line number Diff line change
Expand Up @@ -572,12 +572,27 @@ private fun ContentResolver.resolve(
// RawContacts and Contacts table respectively. Suppress DB exceptions because the where
// clause may contain fields (columns) that are not in the respective tables.
if (includeBlanks) {
contactIds.addAll(
findContactIdsInRawContactsTable(where.inRawContactsTable(), true, cancel)
)
contactIds.addAll(
findContactIdsInContactsTable(where.inContactsTable(), true, cancel)
)
val rawContactsTableWhere = where.toRawContactsTableWhere()
if (rawContactsTableWhere != null) {
// We do not actually need to suppress DB exceptions anymore because we are making
// sure that only RawContacts fields are in rawContactsTableWhere. However, it
// does not hurt to be extra safe... though this will mask programming errors in
// toRawContactsTableWhere by not crashing. Unit tests should cover this though!
contactIds.addAll(
findContactIdsInRawContactsTable(rawContactsTableWhere, true, cancel)
)
}

val contactsTableWhere = where.toContactsTableWhere()
if (contactsTableWhere != null) {
// We do not actually need to suppress DB exceptions anymore because we are making
// sure that only Contacts fields are in contactsTableWhere. However, it does not
// hurt to be extra safe... though this will mask programming errors in
// toContactsTableWhere by not crashing. Unit tests should cover this though!
contactIds.addAll(
findContactIdsInContactsTable(contactsTableWhere, true, cancel)
)
}
}

// If no match, return empty list.
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/contacts/core/Where.kt
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ private fun <F : Field, V : Any?> Sequence<V>.combineWhere(
* a Where composed of one or more GroupsField.
*
* The type [T] is not enforced within the class itself in order to support mutating functions
* such as [contacts.core.util.inRawContactsTable] and [contacts.core.util.inContactsTable].
* such as [contacts.core.util.toRawContactsTableWhere] and [contacts.core.util.toContactsTableWhere].
* This will allow us to construct a Where<X> from a Where<Y>.
*
* ### Binary tree structure
Expand Down Expand Up @@ -840,7 +840,7 @@ fun Any.likeWildcardsEscaped(escapeExpression: String = LIKE_ESCAPE_EXPR): Strin
}

/**
* Exception thrown if the given where is not one of the following valid forms,
* Exception thrown if the given where is NOT one of the following valid forms,
*
* - "field match value"
* - "where combine where"
Expand Down
112 changes: 91 additions & 21 deletions core/src/main/java/contacts/core/util/WhereSubstitutions.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,33 @@ import contacts.core.*
*
* More specifically, this translates the following column names to work with the Contacts table;
*
* - RawContacts.CONTACT_ID -> Contacts._ID
* - Data.CONTACT_ID -> Contacts._ID
*
* This does no translate anything else. So any fields used that does not exist in the Contacts
* table will remain.
* This does not translate anything else. If any fields that does not exist in the Contacts table
* are encountered, then this will return null.
*/
internal fun Where<AbstractDataField>.inContactsTable(): Where<ContactsField> =
copyWithFieldValueSubstitutions(
substituteField = { fieldHolder ->
when (fieldHolder.field) {
// Technically, RawContactsFields.ContactId and Fields.Contact.Id have the same columnName.
// For the sake of OCD and just-in-case, I'm performing this redundant replacement. SUE ME!
RawContactsFields.ContactId, Fields.Contact.Id -> FieldHolder(ContactsFields.Id)
else -> fieldHolder // no substitution
fun Where<AbstractDataField>.toContactsTableWhere(): Where<ContactsField>? =
if (containsField(Fields.DataId.columnName)) {
// The ID column names in all tables are the same. Therefore, we need to make sure that if
// the Data table where is using the Data ID, then there is no corresponding Contacts table
// where to avoid unintentional matching.
null
} else {
val transformedWhere = copyWithFieldValueSubstitutions<AbstractDataField, ContactsField>(
substituteField = { fieldHolder ->
when (fieldHolder.field) {
Fields.Contact.Id -> FieldHolder(ContactsFields.Id)
else -> fieldHolder // no substitution
}
}
)

if (transformedWhere.allFieldsContainedIn(ContactsFields.all.map { it.columnName })) {
transformedWhere
} else {
null
}
)
}

/**
* Converts [this] Data where clause to a where clause that is usable for the RawContacts table.
Expand All @@ -33,18 +43,78 @@ internal fun Where<AbstractDataField>.inContactsTable(): Where<ContactsField> =
*
* - Data.RAW_CONTACT_ID -> RawContacts._ID
*
* This does no translate anything else. So any fields used that does not exist in the RawContacts
* table will remain.
* This does not translate anything else. If any fields that does not exist in the RawContacts table
* are encountered, then this will return null.
*/
internal fun Where<AbstractDataField>.inRawContactsTable(): Where<RawContactsField> =
copyWithFieldValueSubstitutions(
substituteField = { fieldHolder ->
when (fieldHolder.field) {
Fields.RawContact.Id -> FieldHolder(RawContactsFields.Id)
else -> fieldHolder // no substitution
fun Where<AbstractDataField>.toRawContactsTableWhere(): Where<RawContactsField>? =
if (containsField(Fields.DataId.columnName)) {
// The ID column names in all tables are the same. Therefore, we need to make sure that if
// the Data table where is using the Data ID, then there is no corresponding RawContacts
// table where to avoid unintentional matching.
null
} else {
val transformedWhere = copyWithFieldValueSubstitutions<AbstractDataField, RawContactsField>(
substituteField = { fieldHolder ->
when (fieldHolder.field) {
Fields.RawContact.Id -> FieldHolder(RawContactsFields.Id)
// This part is not necessary at all because both Fields.Contact.Id and
// RawContactsFields.ContactId reference RawContactsColumns.CONTACT_ID. This
// essentially does nothing... but it doesn't hurt!
Fields.Contact.Id -> FieldHolder(RawContactsFields.ContactId)
else -> fieldHolder // no substitution
}
}
)

// Make sure that Options columns are not included in the field set check because the
// options in the Data table reference Contacts table options and NOT RawContacts table
// options.
val rawContactsFieldsFromDataFields = RawContactsFields.all.asSequence().minus(
// The RawContactsFields.Options.Id is structurally equal to RawContactsFields.Id so
// we need to make sure we are not subtracting that.
RawContactsFields.Options.all.minus(RawContactsFields.Options.Id)
)
if (transformedWhere.allFieldsContainedIn(
rawContactsFieldsFromDataFields.map { it.columnName }.toSet()
)
) {
transformedWhere
} else {
null
}
)
}

/**
* Returns true if all of the fields in this [Where] is contained in [fieldColumnNames]. Returns
* false if even one field in this [Where] is not in [fieldColumnNames].
*/
private fun Where<Field>.allFieldsContainedIn(fieldColumnNames: Collection<String>): Boolean = run {
if (lhs is FieldHolder) {
// Base case.
fieldColumnNames.contains(lhs.field.columnName)
} else if (lhs is WhereHolder && operator is Operator.Combine && rhs is WhereHolder) {
// Recursive case.
lhs.where.allFieldsContainedIn(fieldColumnNames)
&& rhs.where.allFieldsContainedIn(fieldColumnNames)
} else {
throw InvalidWhereFormException(this)
}
}

/**
* Returns true if this [Where] contains the given [fieldColumnName].
*/
private fun Where<Field>.containsField(fieldColumnName: String): Boolean = run {
if (lhs is FieldHolder) {
// Base case.
lhs.field.columnName == fieldColumnName
} else if (lhs is WhereHolder && operator is Operator.Combine && rhs is WhereHolder) {
// Recursive case.
lhs.where.containsField(fieldColumnName) || rhs.where.containsField(fieldColumnName)
} else {
throw InvalidWhereFormException(this)
}
}

/**
* Returns a copy of this [Where] such that LHS fields are substituted with the output of
Expand Down

0 comments on commit 1f13670

Please sign in to comment.