Trim edit fields that don't contain printable values.
When persisting edit changes, trim data rows that have no
printable characters. Also wrote unit tests for new method
and building of IM intents to check Uri escaping.
Fixes http://b/2173811
diff --git a/src/com/android/contacts/ContactsUtils.java b/src/com/android/contacts/ContactsUtils.java
index 5f003de..1e3b8ad 100644
--- a/src/com/android/contacts/ContactsUtils.java
+++ b/src/com/android/contacts/ContactsUtils.java
@@ -416,4 +416,12 @@
Uri.fromParts("sms", phoneNumber.toString(), null));
context.startActivity(intent);
}
+
+ /**
+ * Test if the given {@link CharSequence} contains any graphic characters,
+ * first checking {@link TextUtils#isEmpty(CharSequence)} to handle null.
+ */
+ public static boolean isGraphic(CharSequence str) {
+ return !TextUtils.isEmpty(str) && TextUtils.isGraphic(str);
+ }
}
diff --git a/src/com/android/contacts/model/EntityModifier.java b/src/com/android/contacts/model/EntityModifier.java
index 5700df2..519ff80 100644
--- a/src/com/android/contacts/model/EntityModifier.java
+++ b/src/com/android/contacts/model/EntityModifier.java
@@ -16,6 +16,7 @@
package com.android.contacts.model;
+import com.android.contacts.ContactsUtils;
import com.android.contacts.model.ContactsSource.DataKind;
import com.android.contacts.model.ContactsSource.EditField;
import com.android.contacts.model.ContactsSource.EditType;
@@ -412,7 +413,7 @@
for (EditField field : kind.fieldList) {
// If any field has values, we're not empty
final String value = values.getAsString(field.column);
- if (!TextUtils.isEmpty(value)) {
+ if (ContactsUtils.isGraphic(value)) {
hasValues = true;
}
}
@@ -437,12 +438,12 @@
final ValuesDelta child = state.getPrimaryEntry(StructuredName.CONTENT_ITEM_TYPE);
final String name = extras.getString(Insert.NAME);
- if (!TextUtils.isEmpty(name) && TextUtils.isGraphic(name)) {
+ if (ContactsUtils.isGraphic(name)) {
child.put(StructuredName.GIVEN_NAME, name);
}
final String phoneticName = extras.getString(Insert.PHONETIC_NAME);
- if (!TextUtils.isEmpty(phoneticName) && TextUtils.isGraphic(phoneticName)) {
+ if (ContactsUtils.isGraphic(phoneticName)) {
child.put(StructuredName.PHONETIC_GIVEN_NAME, phoneticName);
}
}
diff --git a/src/com/android/contacts/ui/widget/GenericEditorView.java b/src/com/android/contacts/ui/widget/GenericEditorView.java
index 26d4aab..8b50551 100644
--- a/src/com/android/contacts/ui/widget/GenericEditorView.java
+++ b/src/com/android/contacts/ui/widget/GenericEditorView.java
@@ -16,6 +16,7 @@
package com.android.contacts.ui.widget;
+import com.android.contacts.ContactsUtils;
import com.android.contacts.R;
import com.android.contacts.model.Editor;
import com.android.contacts.model.EntityDelta;
@@ -222,7 +223,7 @@
});
// Hide field when empty and optional value
- final boolean couldHide = (TextUtils.isEmpty(value) && field.optional);
+ final boolean couldHide = (!ContactsUtils.isGraphic(value) && field.optional);
final boolean willHide = (mHideOptional && couldHide);
fieldView.setVisibility(willHide ? View.GONE : View.VISIBLE);
fieldView.setEnabled(enabled);
@@ -255,7 +256,7 @@
builder.setPositiveButton(android.R.string.ok, new DialogInterface.OnClickListener() {
public void onClick(DialogInterface dialog, int which) {
final String customText = customType.getText().toString().trim();
- if (!TextUtils.isEmpty(customText)) {
+ if (ContactsUtils.isGraphic(customText)) {
// Now we're sure it's ok to actually change the type value.
mType = mPendingType;
mPendingType = null;
diff --git a/tests/src/com/android/contacts/ContactsUtilsTests.java b/tests/src/com/android/contacts/ContactsUtilsTests.java
new file mode 100644
index 0000000..01a1ef4
--- /dev/null
+++ b/tests/src/com/android/contacts/ContactsUtilsTests.java
@@ -0,0 +1,101 @@
+/*
+ * Copyright (C) 2009 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.contacts;
+
+import android.content.ContentValues;
+import android.content.Intent;
+import android.net.Uri;
+import android.provider.ContactsContract.CommonDataKinds.Email;
+import android.provider.ContactsContract.CommonDataKinds.Im;
+import android.test.AndroidTestCase;
+import android.test.suitebuilder.annotation.LargeTest;
+
+/**
+ * Tests for {@link ContactsUtils}.
+ */
+@LargeTest
+public class ContactsUtilsTests extends AndroidTestCase {
+ private static final String TEST_ADDRESS = "user@example.org";
+ private static final String TEST_PROTOCOL = "prot%col";
+
+ public void testImIntent() throws Exception {
+ // Normal IM is appended as path
+ final ContentValues values = new ContentValues();
+ values.put(Im.MIMETYPE, Im.CONTENT_ITEM_TYPE);
+ values.put(Im.TYPE, Im.TYPE_HOME);
+ values.put(Im.PROTOCOL, Im.PROTOCOL_GOOGLE_TALK);
+ values.put(Im.DATA, TEST_ADDRESS);
+
+ final Intent intent = ContactsUtils.buildImIntent(values);
+ assertEquals(Intent.ACTION_SENDTO, intent.getAction());
+
+ final Uri data = intent.getData();
+ assertEquals("imto", data.getScheme());
+ assertEquals("gtalk", data.getAuthority());
+ assertEquals(TEST_ADDRESS, data.getPathSegments().get(0));
+ }
+
+ public void testImIntentCustom() throws Exception {
+ // Custom IM types have encoded authority
+ final ContentValues values = new ContentValues();
+ values.put(Im.MIMETYPE, Im.CONTENT_ITEM_TYPE);
+ values.put(Im.TYPE, Im.TYPE_HOME);
+ values.put(Im.PROTOCOL, Im.PROTOCOL_CUSTOM);
+ values.put(Im.CUSTOM_PROTOCOL, TEST_PROTOCOL);
+ values.put(Im.DATA, TEST_ADDRESS);
+
+ final Intent intent = ContactsUtils.buildImIntent(values);
+ assertEquals(Intent.ACTION_SENDTO, intent.getAction());
+
+ final Uri data = intent.getData();
+ assertEquals("imto", data.getScheme());
+ assertEquals(TEST_PROTOCOL, data.getAuthority());
+ assertEquals(TEST_ADDRESS, data.getPathSegments().get(0));
+ }
+
+ public void testImEmailIntent() throws Exception {
+ // Email addresses are treated as Google Talk entries
+ final ContentValues values = new ContentValues();
+ values.put(Email.MIMETYPE, Email.CONTENT_ITEM_TYPE);
+ values.put(Email.TYPE, Email.TYPE_HOME);
+ values.put(Email.DATA, TEST_ADDRESS);
+
+ final Intent intent = ContactsUtils.buildImIntent(values);
+ assertEquals(Intent.ACTION_SENDTO, intent.getAction());
+
+ final Uri data = intent.getData();
+ assertEquals("imto", data.getScheme());
+ assertEquals("gtalk", data.getAuthority());
+ assertEquals(TEST_ADDRESS, data.getPathSegments().get(0));
+ }
+
+ public void testIsGraphicNull() throws Exception {
+ assertFalse(ContactsUtils.isGraphic(null));
+ }
+
+ public void testIsGraphicEmpty() throws Exception {
+ assertFalse(ContactsUtils.isGraphic(""));
+ }
+
+ public void testIsGraphicSpaces() throws Exception {
+ assertFalse(ContactsUtils.isGraphic(" "));
+ }
+
+ public void testIsGraphicPunctuation() throws Exception {
+ assertTrue(ContactsUtils.isGraphic("."));
+ }
+}
diff --git a/tests/src/com/android/contacts/EntityModifierTests.java b/tests/src/com/android/contacts/EntityModifierTests.java
index d13e27f..489bf04 100644
--- a/tests/src/com/android/contacts/EntityModifierTests.java
+++ b/tests/src/com/android/contacts/EntityModifierTests.java
@@ -52,6 +52,8 @@
public class EntityModifierTests extends AndroidTestCase {
public static final String TAG = "EntityModifierTests";
+ public static final long VER_FIRST = 100;
+
private static final long TEST_ID = 4;
private static final String TEST_PHONE = "218-555-1212";
@@ -346,6 +348,59 @@
}
}
+ public void testTrimEmptySpaces() {
+ final ContactsSource source = getSource();
+ final DataKind kindPhone = source.getKindForMimetype(Phone.CONTENT_ITEM_TYPE);
+ final EditType typeHome = EntityModifier.getType(kindPhone, Phone.TYPE_HOME);
+
+ // Test row that has type values, but values are spaces
+ final EntityDelta state = EntitySetTests.buildBeforeEntity(TEST_ID, VER_FIRST);
+ final ValuesDelta values = EntityModifier.insertChild(state, kindPhone, typeHome);
+ values.put(Phone.NUMBER, " ");
+
+ // Build diff, expecting insert for data row and update enforcement
+ EntitySetTests.assertDiffPattern(state,
+ EntitySetTests.buildAssertVersion(VER_FIRST),
+ EntitySetTests.buildUpdateAggregationSuspended(),
+ EntitySetTests.buildOper(Data.CONTENT_URI, TYPE_INSERT,
+ EntitySetTests.buildDataInsert(values, TEST_ID)),
+ EntitySetTests.buildUpdateAggregationDefault());
+
+ // Trim empty rows and try again, expecting delete of overall contact
+ EntityModifier.trimEmpty(state, source);
+ EntitySetTests.assertDiffPattern(state,
+ EntitySetTests.buildAssertVersion(VER_FIRST),
+ EntitySetTests.buildDelete(RawContacts.CONTENT_URI));
+ }
+
+ public void testTrimLeaveValid() {
+ final ContactsSource source = getSource();
+ final DataKind kindPhone = source.getKindForMimetype(Phone.CONTENT_ITEM_TYPE);
+ final EditType typeHome = EntityModifier.getType(kindPhone, Phone.TYPE_HOME);
+
+ // Test row that has type values with valid number
+ final EntityDelta state = EntitySetTests.buildBeforeEntity(TEST_ID, VER_FIRST);
+ final ValuesDelta values = EntityModifier.insertChild(state, kindPhone, typeHome);
+ values.put(Phone.NUMBER, TEST_PHONE);
+
+ // Build diff, expecting insert for data row and update enforcement
+ EntitySetTests.assertDiffPattern(state,
+ EntitySetTests.buildAssertVersion(VER_FIRST),
+ EntitySetTests.buildUpdateAggregationSuspended(),
+ EntitySetTests.buildOper(Data.CONTENT_URI, TYPE_INSERT,
+ EntitySetTests.buildDataInsert(values, TEST_ID)),
+ EntitySetTests.buildUpdateAggregationDefault());
+
+ // Trim empty rows and try again, expecting no differences
+ EntityModifier.trimEmpty(state, source);
+ EntitySetTests.assertDiffPattern(state,
+ EntitySetTests.buildAssertVersion(VER_FIRST),
+ EntitySetTests.buildUpdateAggregationSuspended(),
+ EntitySetTests.buildOper(Data.CONTENT_URI, TYPE_INSERT,
+ EntitySetTests.buildDataInsert(values, TEST_ID)),
+ EntitySetTests.buildUpdateAggregationDefault());
+ }
+
public void testTrimEmptyUntouched() {
final ContactsSource source = getSource();
final DataKind kindPhone = source.getKindForMimetype(Phone.CONTENT_ITEM_TYPE);
diff --git a/tests/src/com/android/contacts/EntitySetTests.java b/tests/src/com/android/contacts/EntitySetTests.java
index 94477ba..26f4184 100644
--- a/tests/src/com/android/contacts/EntitySetTests.java
+++ b/tests/src/com/android/contacts/EntitySetTests.java
@@ -27,6 +27,7 @@
import com.android.contacts.model.EntityModifier;
import com.android.contacts.model.EntitySet;
import com.android.contacts.model.EntityDelta.ValuesDelta;
+import com.google.android.collect.Lists;
import android.content.ContentProviderOperation;
import android.content.ContentValues;
@@ -89,20 +90,20 @@
return source;
}
- private ContentValues getValues(ContentProviderOperation operation)
+ static ContentValues getValues(ContentProviderOperation operation)
throws NoSuchFieldException, IllegalAccessException {
final Field field = ContentProviderOperation.class.getDeclaredField("mValues");
field.setAccessible(true);
return (ContentValues) field.get(operation);
}
- protected static EntityDelta getUpdate(long rawContactId) {
+ static EntityDelta getUpdate(long rawContactId) {
final Entity before = EntityDeltaTests.getEntity(rawContactId,
EntityDeltaTests.TEST_PHONE_ID);
return EntityDelta.fromBefore(before);
}
- protected static EntityDelta getInsert() {
+ static EntityDelta getInsert() {
final ContentValues after = new ContentValues();
after.put(RawContacts.ACCOUNT_NAME, EntityDeltaTests.TEST_ACCOUNT_NAME);
after.put(RawContacts.SEND_TO_VOICEMAIL, 1);
@@ -111,7 +112,7 @@
return new EntityDelta(values);
}
- protected static EntitySet buildSet(EntityDelta... deltas) {
+ static EntitySet buildSet(EntityDelta... deltas) {
final EntitySet set = EntitySet.fromSingle(deltas[0]);
for (int i = 1; i < deltas.length; i++) {
set.add(deltas[i]);
@@ -119,7 +120,7 @@
return set;
}
- protected static EntityDelta buildBeforeEntity(long rawContactId, long version,
+ static EntityDelta buildBeforeEntity(long rawContactId, long version,
ContentValues... entries) {
// Build an existing contact read from database
final ContentValues contact = new ContentValues();
@@ -132,7 +133,7 @@
return EntityDelta.fromBefore(before);
}
- protected static EntityDelta buildAfterEntity(ContentValues... entries) {
+ static EntityDelta buildAfterEntity(ContentValues... entries) {
// Build an existing contact read from database
final ContentValues contact = new ContentValues();
contact.put(RawContacts.ACCOUNT_TYPE, TEST_ACCOUNT);
@@ -143,11 +144,11 @@
return after;
}
- protected static ContentValues buildPhone(long phoneId) {
+ static ContentValues buildPhone(long phoneId) {
return buildPhone(phoneId, Long.toString(phoneId));
}
- protected static ContentValues buildPhone(long phoneId, String value) {
+ static ContentValues buildPhone(long phoneId, String value) {
final ContentValues values = new ContentValues();
values.put(Data._ID, phoneId);
values.put(Data.MIMETYPE, Phone.CONTENT_ITEM_TYPE);
@@ -156,7 +157,7 @@
return values;
}
- protected static ContentValues buildEmail(long emailId) {
+ static ContentValues buildEmail(long emailId) {
final ContentValues values = new ContentValues();
values.put(Data._ID, emailId);
values.put(Data.MIMETYPE, Email.CONTENT_ITEM_TYPE);
@@ -165,19 +166,29 @@
return values;
}
- protected static void insertPhone(EntitySet set, long rawContactId, ContentValues values) {
+ static void insertPhone(EntitySet set, long rawContactId, ContentValues values) {
final EntityDelta match = set.getByRawContactId(rawContactId);
match.addEntry(ValuesDelta.fromAfter(values));
}
- protected static ValuesDelta getPhone(EntitySet set, long rawContactId, long dataId) {
+ static ValuesDelta getPhone(EntitySet set, long rawContactId, long dataId) {
final EntityDelta match = set.getByRawContactId(rawContactId);
return match.getEntry(dataId);
}
- protected void assertDiffPattern(EntitySet set, ContentProviderOperation... pattern) {
- final ArrayList<ContentProviderOperation> diff = set.buildDiff();
+ static void assertDiffPattern(EntityDelta delta, ContentProviderOperation... pattern) {
+ final ArrayList<ContentProviderOperation> diff = Lists.newArrayList();
+ delta.buildAssert(diff);
+ delta.buildDiff(diff);
+ assertDiffPattern(diff, pattern);
+ }
+ static void assertDiffPattern(EntitySet set, ContentProviderOperation... pattern) {
+ assertDiffPattern(set.buildDiff(), pattern);
+ }
+
+ static void assertDiffPattern(ArrayList<ContentProviderOperation> diff,
+ ContentProviderOperation... pattern) {
assertEquals("Unexpected operations", pattern.length, diff.size());
for (int i = 0; i < pattern.length; i++) {
final ContentProviderOperation expected = pattern[i];
@@ -207,7 +218,7 @@
}
}
- protected static String getStringForType(int type) {
+ static String getStringForType(int type) {
switch (type) {
case TYPE_ASSERT: return "TYPE_ASSERT";
case TYPE_INSERT: return "TYPE_INSERT";
@@ -217,48 +228,48 @@
}
}
- protected static ContentProviderOperation buildAssertVersion(long version) {
+ static ContentProviderOperation buildAssertVersion(long version) {
final ContentValues values = new ContentValues();
values.put(RawContacts.VERSION, version);
return buildOper(RawContacts.CONTENT_URI, TYPE_ASSERT, values);
}
- protected static ContentProviderOperation buildAggregationModeUpdate(int mode) {
+ static ContentProviderOperation buildAggregationModeUpdate(int mode) {
final ContentValues values = new ContentValues();
values.put(RawContacts.AGGREGATION_MODE, mode);
return buildOper(RawContacts.CONTENT_URI, TYPE_UPDATE, values);
}
- protected static ContentProviderOperation buildUpdateAggregationSuspended() {
+ static ContentProviderOperation buildUpdateAggregationSuspended() {
return buildAggregationModeUpdate(RawContacts.AGGREGATION_MODE_SUSPENDED);
}
- protected static ContentProviderOperation buildUpdateAggregationDefault() {
+ static ContentProviderOperation buildUpdateAggregationDefault() {
return buildAggregationModeUpdate(RawContacts.AGGREGATION_MODE_DEFAULT);
}
- protected static ContentProviderOperation buildUpdateAggregationKeepTogether(long rawContactId) {
+ static ContentProviderOperation buildUpdateAggregationKeepTogether(long rawContactId) {
final ContentValues values = new ContentValues();
values.put(AggregationExceptions.RAW_CONTACT_ID1, rawContactId);
values.put(AggregationExceptions.TYPE, AggregationExceptions.TYPE_KEEP_TOGETHER);
return buildOper(AggregationExceptions.CONTENT_URI, TYPE_UPDATE, values);
}
- protected static ContentValues buildDataInsert(ValuesDelta values, long rawContactId) {
+ static ContentValues buildDataInsert(ValuesDelta values, long rawContactId) {
final ContentValues insertValues = values.getCompleteValues();
insertValues.put(Data.RAW_CONTACT_ID, rawContactId);
return insertValues;
}
- protected static ContentProviderOperation buildDelete(Uri uri) {
+ static ContentProviderOperation buildDelete(Uri uri) {
return buildOper(uri, TYPE_DELETE, (ContentValues)null);
}
- protected static ContentProviderOperation buildOper(Uri uri, int type, ValuesDelta values) {
+ static ContentProviderOperation buildOper(Uri uri, int type, ValuesDelta values) {
return buildOper(uri, type, values.getCompleteValues());
}
- protected static ContentProviderOperation buildOper(Uri uri, int type, ContentValues values) {
+ static ContentProviderOperation buildOper(Uri uri, int type, ContentValues values) {
switch (type) {
case TYPE_ASSERT:
return ContentProviderOperation.newAssertQuery(uri).withValues(values).build();
@@ -272,7 +283,7 @@
return null;
}
- protected static Long getVersion(EntitySet set, Long rawContactId) {
+ static Long getVersion(EntitySet set, Long rawContactId) {
return set.getByRawContactId(rawContactId).getValues().getAsLong(RawContacts.VERSION);
}
@@ -280,7 +291,7 @@
* Count number of {@link AggregationExceptions} updates contained in the
* given list of {@link ContentProviderOperation}.
*/
- protected static int countExceptionUpdates(ArrayList<ContentProviderOperation> diff) {
+ static int countExceptionUpdates(ArrayList<ContentProviderOperation> diff) {
int updateCount = 0;
for (ContentProviderOperation oper : diff) {
if (AggregationExceptions.CONTENT_URI.equals(oper.getUri())