+ * The key, value will not add to {@code cv} if value is null. + * + * @return true if values are different. {@code assumeDiff} indicates if values can be assumed + * different in which case no comparison is needed. */ - boolean setStringValueAndCheckIfDiff(ContentValues cv, String key, String value, - boolean assumeDiff, int index) { - cv.put(key, value); - String valueFromCursor = mCursor.getString(index); + boolean setStringValueAndCheckIfDiff( + ContentValues cv, String key, String value, boolean assumeDiff, int index) { + String valueFromLocalCache = mApnData.getString(index); if (VDBG) { Log.d(TAG, "setStringValueAndCheckIfDiff: assumeDiff: " + assumeDiff + " key: " + key + " value: '" + value - + "' valueFromCursor: '" + valueFromCursor + "'"); + + "' valueFromDb: '" + valueFromLocalCache + "'"); } - return assumeDiff - || !((TextUtils.isEmpty(value) && TextUtils.isEmpty(valueFromCursor)) - || (value != null && value.equals(valueFromCursor))); + boolean isDiff = assumeDiff + || !((TextUtils.isEmpty(value) && TextUtils.isEmpty(valueFromLocalCache)) + || (value != null && value.equals(valueFromLocalCache))); + + if (isDiff && value != null) { + cv.put(key, value); + } + return isDiff; } /** - * Add key, value to cv and compare the value against the value at index in mCursor. Return true - * if values are different. assumeDiff indicates if values can be assumed different in which - * case no comparison is needed. - * @return true if value is different from the value at index in mCursor + * Add key, value to {@code cv} and compare the value against the value at index in + * {@link #mApnData}. + * + * @return true if values are different. {@code assumeDiff} indicates if values can be assumed + * different in which case no comparison is needed. */ - boolean setIntValueAndCheckIfDiff(ContentValues cv, String key, int value, - boolean assumeDiff, int index) { - cv.put(key, value); - int valueFromCursor = mCursor.getInt(index); + boolean setIntValueAndCheckIfDiff( + ContentValues cv, String key, int value, boolean assumeDiff, int index) { + Integer valueFromLocalCache = mApnData.getInteger(index); if (VDBG) { Log.d(TAG, "setIntValueAndCheckIfDiff: assumeDiff: " + assumeDiff + " key: " + key + " value: '" + value - + "' valueFromCursor: '" + valueFromCursor + "'"); + + "' valueFromDb: '" + valueFromLocalCache + "'"); } - return assumeDiff || value != valueFromCursor; + + boolean isDiff = assumeDiff || value != valueFromLocalCache; + if (isDiff) { + cv.put(key, value); + } + return isDiff; } /** - * Check the key fields' validity and save if valid. - * @param force save even if the fields are not valid, if the app is - * being suspended - * @return true if there's no error + * Validates the apn data and save it to the database if it's valid. + * + *
+ * A dialog with error message will be displayed if the APN data is invalid.
+ *
+ * @return true if there is no error
*/
- private boolean validateAndSave(boolean force) {
- // nothing to do if it's a read only APN
+ @VisibleForTesting
+ boolean validateAndSaveApnData() {
+ // Nothing to do if it's a read only APN
if (mReadOnlyApn) {
return true;
}
@@ -868,21 +857,9 @@ public class ApnEditor extends SettingsPreferenceFragment
String mcc = checkNotSet(mMcc.getText());
String mnc = checkNotSet(mMnc.getText());
- if (getErrorMsg() != null && !force) {
- ErrorDialog.showError(this);
- return false;
- }
-
- if (!mCursor.moveToFirst()) {
- Log.w(TAG,
- "Could not go to the first row in the Cursor when saving data.");
- return false;
- }
-
- // If it's a new APN and a name or apn haven't been entered, then erase the entry
- if (force && mNewApn && name.length() < 1 && apn.length() < 1) {
- getContentResolver().delete(mUri, null, null);
- mUri = null;
+ String errorMsg = validateApnData();
+ if (errorMsg != null) {
+ showError();
return false;
}
@@ -890,12 +867,9 @@ public class ApnEditor extends SettingsPreferenceFragment
// call update() if it's a new APN. If not, check if any field differs from the db value;
// if any diff is found update() should be called
boolean callUpdate = mNewApn;
-
- // Add a dummy name "Untitled", if the user exits the screen without adding a name but
- // entered other information worth keeping.
callUpdate = setStringValueAndCheckIfDiff(values,
Telephony.Carriers.NAME,
- name.length() < 1 ? getResources().getString(R.string.untitled_apn) : name,
+ name,
callUpdate,
NAME_INDEX);
@@ -1054,15 +1028,38 @@ public class ApnEditor extends SettingsPreferenceFragment
values.put(Telephony.Carriers.EDITED, Telephony.Carriers.USER_EDITED);
if (callUpdate) {
- getContentResolver().update(mUri, values, null, null);
+ final Uri uri = mApnData.getUri() == null ? mCarrierUri : mApnData.getUri();
+ updateApnDataToDatabase(uri, values);
} else {
- if (VDBG) Log.d(TAG, "validateAndSave: not calling update()");
+ if (VDBG) Log.d(TAG, "validateAndSaveApnData: not calling update()");
}
return true;
}
- private String getErrorMsg() {
+ private void updateApnDataToDatabase(Uri uri, ContentValues values) {
+ ThreadUtils.postOnBackgroundThread(() -> {
+ if (uri.equals(mCarrierUri)) {
+ // Add a new apn to the database
+ final Uri newUri = getContentResolver().insert(mCarrierUri, values);
+ if (newUri == null) {
+ Log.e(TAG, "Can't add a new apn to database " + mCarrierUri);
+ }
+ } else {
+ // Update the existing apn
+ getContentResolver().update(
+ uri, values, null /* where */, null /* selection Args */);
+ }
+ });
+ }
+
+ /**
+ * Validates whether the apn data is valid.
+ *
+ * @return An error message if the apn data is invalid, otherwise return null.
+ */
+ @VisibleForTesting
+ String validateApnData() {
String errorMsg = null;
String name = checkNotSet(mName.getText());
@@ -1070,14 +1067,14 @@ public class ApnEditor extends SettingsPreferenceFragment
String mcc = checkNotSet(mMcc.getText());
String mnc = checkNotSet(mMnc.getText());
- if (name.length() < 1) {
- errorMsg = mRes.getString(R.string.error_name_empty);
- } else if (apn.length() < 1) {
- errorMsg = mRes.getString(R.string.error_apn_empty);
- } else if (mcc.length() != 3) {
- errorMsg = mRes.getString(R.string.error_mcc_not3);
- } else if ((mnc.length() & 0xFFFE) != 2) {
- errorMsg = mRes.getString(R.string.error_mnc_not23);
+ if (TextUtils.isEmpty(name)) {
+ errorMsg = getResources().getString(R.string.error_name_empty);
+ } else if (TextUtils.isEmpty(apn)) {
+ errorMsg = getResources().getString(R.string.error_apn_empty);
+ } else if (mcc == null || mcc.length() != 3) {
+ errorMsg = getResources().getString(R.string.error_mcc_not3);
+ } else if ((mnc == null || (mnc.length() & 0xFFFE) != 2)) {
+ errorMsg = getResources().getString(R.string.error_mnc_not23);
}
if (errorMsg == null) {
@@ -1088,13 +1085,13 @@ public class ApnEditor extends SettingsPreferenceFragment
StringBuilder stringBuilder = new StringBuilder();
for (String type : mReadOnlyApnTypes) {
stringBuilder.append(type).append(", ");
- Log.d(TAG, "getErrorMsg: appending type: " + type);
+ Log.d(TAG, "validateApnData: appending type: " + type);
}
// remove last ", "
if (stringBuilder.length() >= 2) {
stringBuilder.delete(stringBuilder.length() - 2, stringBuilder.length());
}
- errorMsg = String.format(mRes.getString(R.string.error_adding_apn_type),
+ errorMsg = String.format(getResources().getString(R.string.error_adding_apn_type),
stringBuilder);
}
}
@@ -1102,9 +1099,16 @@ public class ApnEditor extends SettingsPreferenceFragment
return errorMsg;
}
+ @VisibleForTesting
+ void showError() {
+ ErrorDialog.showError(this);
+ }
+
private void deleteApn() {
- getContentResolver().delete(mUri, null, null);
- finish();
+ if (mApnData.getUri() != null) {
+ getContentResolver().delete(mApnData.getUri(), null, null);
+ mApnData = new ApnData(sProjection.length);
+ }
}
private String starify(String value) {
@@ -1119,20 +1123,21 @@ public class ApnEditor extends SettingsPreferenceFragment
}
}
+ /**
+ * Returns {@link #sNotSet} if the given string {@code value} is null or empty. The string
+ * {@link #sNotSet} typically used as the default display when an entry in the preference is
+ * null or empty.
+ */
private String checkNull(String value) {
- if (value == null || value.length() == 0) {
- return sNotSet;
- } else {
- return value;
- }
+ return TextUtils.isEmpty(value) ? sNotSet : value;
}
+ /**
+ * Returns null if the given string {@code value} equals to {@link #sNotSet}. This method
+ * should be used when convert a string value from preference to database.
+ */
private String checkNotSet(String value) {
- if (value == null || value.equals(sNotSet)) {
- return "";
- } else {
- return value;
- }
+ return sNotSet.equals(value) ? null : value;
}
private String getUserEnteredApnType() {
@@ -1176,7 +1181,7 @@ public class ApnEditor extends SettingsPreferenceFragment
@Override
public Dialog onCreateDialog(Bundle savedInstanceState) {
- String msg = ((ApnEditor) getTargetFragment()).getErrorMsg();
+ String msg = ((ApnEditor) getTargetFragment()).validateApnData();
return new AlertDialog.Builder(getContext())
.setTitle(R.string.error_title)
@@ -1191,10 +1196,19 @@ public class ApnEditor extends SettingsPreferenceFragment
}
}
- public static class InvalidTypeException extends RuntimeException {
- InvalidTypeException(String msg) {
- super(msg);
+ private ApnData getApnDataFromUri(Uri uri) {
+ ApnData apnData;
+ try (Cursor cursor = getActivity().managedQuery(
+ uri, sProjection, null /* selection */, null /* sortOrder */)) {
+ cursor.moveToFirst();
+ apnData = new ApnData(uri, cursor);
}
+
+ if (apnData == null) {
+ Log.d(TAG, "Can't get apnData from Uri " + uri);
+ }
+
+ return apnData;
}
@VisibleForTesting
@@ -1243,34 +1257,17 @@ public class ApnEditor extends SettingsPreferenceFragment
mUri = uri;
}
- Integer getInteger(int index) throws InvalidTypeException {
- if (!isValidTypeOrNull(mData[index], Integer.class)) {
- throwInvalidTypeException(Integer.class, mData[index].getClass());
- }
+ Integer getInteger(int index) {
return (Integer) mData[index];
}
- Integer getInteger(int index, Integer defaultValue) throws InvalidTypeException {
+ Integer getInteger(int index, Integer defaultValue) {
Integer val = getInteger(index);
return val == null ? defaultValue : val;
}
- String getString(int index) throws InvalidTypeException {
- if (!isValidTypeOrNull(mData[index], String.class)) {
- throwInvalidTypeException(String.class, mData[index].getClass());
- }
+ String getString(int index) {
return (String) mData[index];
}
-
- private boolean isValidTypeOrNull(Object obj, Class expectedClass) {
- return obj == null || expectedClass.isInstance(obj);
- }
-
- private void throwInvalidTypeException(Class expectedClass, Class actualClass) {
- throw new InvalidTypeException(
- String.format(
- "Type mismatched, want %s, but is %s", expectedClass, actualClass));
- }
}
-
}
diff --git a/tests/robotests/src/com/android/settings/ApnEditorTest.java b/tests/robotests/src/com/android/settings/ApnEditorTest.java
index eb9955adb3e..aca460f89ae 100644
--- a/tests/robotests/src/com/android/settings/ApnEditorTest.java
+++ b/tests/robotests/src/com/android/settings/ApnEditorTest.java
@@ -16,27 +16,73 @@
package com.android.settings;
-import static junit.framework.Assert.assertEquals;
-import static junit.framework.Assert.assertNull;
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import android.app.Activity;
+import android.content.ContentResolver;
+import android.content.ContentValues;
+import android.content.Context;
+import android.content.res.Resources;
import android.database.Cursor;
import android.net.Uri;
+import android.support.v14.preference.MultiSelectListPreference;
+import android.support.v14.preference.SwitchPreference;
+import android.support.v7.preference.EditTextPreference;
+import android.support.v7.preference.ListPreference;
+import android.view.KeyEvent;
+import android.view.Menu;
+import android.view.MenuItem;
+import android.view.View;
import com.android.settings.ApnEditor.ApnData;
-import com.android.settings.ApnEditor.InvalidTypeException;
import com.android.settings.testutils.SettingsRobolectricTestRunner;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
import org.mockito.Mock;
+import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
+import org.robolectric.Robolectric;
+import org.robolectric.RuntimeEnvironment;
@RunWith(SettingsRobolectricTestRunner.class)
public class ApnEditorTest {
+ private static final Object[] APN_DATA = new Object[] {
+ 0, /* ID */
+ "apn_name" /* apn name */,
+ "apn.com" /* apn */,
+ "" /* proxy */,
+ "" /* port */,
+ "" /* username */,
+ "" /* server */,
+ "" /* password */,
+ "" /* MMSC */,
+ "123" /* MCC */,
+ "456" /* MNC */,
+ "123456" /* operator numeric */,
+ "" /* MMS proxy */,
+ "" /* MMS port */,
+ 0 /* Authentication type */,
+ "default,supl,ia" /* APN type */,
+ "IPv6" /* APN protocol */,
+ 1 /* APN enable/disable */,
+ 0 /* Bearer */,
+ 0 /* Bearer BITMASK*/,
+ "IPv4" /* APN roaming protocol */,
+ "None" /* MVNO type */,
+ "", /* MVNO value */
+ };
+
private static final int CURSOR_INTEGER_INDEX = 0;
private static final int CURSOR_STRING_INDEX = 1;
@@ -45,10 +91,331 @@ public class ApnEditorTest {
@Mock
private Cursor mCursor;
+ @Captor
+ private ArgumentCaptor