Refresh user settings on AVD edit
Cache user settings in AvdInfo.
Fixes: 336324982
Test: manual
Change-Id: I8726053326ecd5dfda1c71b701ce0fad47d30228
diff --git a/device-provisioner/src/main/com/android/sdklib/deviceprovisioner/LocalEmulatorProvisionerPlugin.kt b/device-provisioner/src/main/com/android/sdklib/deviceprovisioner/LocalEmulatorProvisionerPlugin.kt
index aaf608e..2575c88 100644
--- a/device-provisioner/src/main/com/android/sdklib/deviceprovisioner/LocalEmulatorProvisionerPlugin.kt
+++ b/device-provisioner/src/main/com/android/sdklib/deviceprovisioner/LocalEmulatorProvisionerPlugin.kt
@@ -35,7 +35,7 @@
import com.android.sdklib.devices.Abi
import com.android.sdklib.internal.avd.AvdInfo
import com.android.sdklib.internal.avd.AvdInfo.AvdStatus
-import com.android.sdklib.internal.avd.AvdManager
+import com.android.sdklib.internal.avd.AvdManager.USER_SETTINGS_INI_PREFERRED_ABI
import com.android.sdklib.internal.avd.HardwareProperties
import com.google.wireless.android.sdk.stats.DeviceInfo
import com.intellij.icons.AllIcons
@@ -297,6 +297,10 @@
logger.debug { "${logName()} Processing: $message" }
when (message) {
is AvdInfoUpdate -> {
+ if (!activeAvdInfo.isSameMetadata(message.avdInfo)) {
+ activeAvdInfo = activeAvdInfo.copyMetadata(message.avdInfo)
+ properties = properties.toBuilder().apply { setAvdInfo(activeAvdInfo) }.build()
+ }
if (connectedDevice == null) {
if (activeAvdInfo != message.avdInfo) {
activeAvdInfo = message.avdInfo
@@ -794,8 +798,7 @@
density = avdInfo.density
resolution = avdInfo.resolution
isDebuggable = !avdInfo.hasPlayStore()
- preferredAbi =
- avdInfo.parseUserSettingsFile(null)?.get(AvdManager.USER_SETTINGS_INI_PREFERRED_ABI)
+ preferredAbi = avdInfo.userSettings[USER_SETTINGS_INI_PREFERRED_ABI]
avdConfigProperties.putAll(avdInfo.properties)
}
diff --git a/device-provisioner/src/test/com/android/sdklib/deviceprovisioner/FakeAvdManager.kt b/device-provisioner/src/test/com/android/sdklib/deviceprovisioner/FakeAvdManager.kt
index 6b949d1..77c8f91 100644
--- a/device-provisioner/src/test/com/android/sdklib/deviceprovisioner/FakeAvdManager.kt
+++ b/device-provisioner/src/test/com/android/sdklib/deviceprovisioner/FakeAvdManager.kt
@@ -36,6 +36,12 @@
val avds = mutableListOf<AvdInfo>()
val runningDevices = mutableSetOf<FakeEmulatorConsole>()
var avdIndex = 1
+ var avdEditor: (AvdInfo) -> AvdInfo = { avdInfo: AvdInfo ->
+ avdInfo.copy(
+ properties =
+ avdInfo.properties + (AvdManager.AVD_INI_DISPLAY_NAME to avdInfo.displayName + " Edited")
+ )
+ }
override suspend fun rescanAvds(): List<AvdInfo> = synchronized(avds) { avds.toList() }
@@ -70,6 +76,7 @@
AvdManager.AVD_INI_TAG_ID to tag.id,
AvdManager.AVD_INI_TAG_DISPLAY to tag.display,
),
+ null,
avdStatus,
)
}
@@ -77,12 +84,7 @@
override suspend fun editAvd(avdInfo: AvdInfo): AvdInfo? =
synchronized(avds) {
avds.remove(avdInfo)
- val newAvdInfo =
- avdInfo.copy(
- properties =
- avdInfo.properties +
- (AvdManager.AVD_INI_DISPLAY_NAME to avdInfo.displayName + " Edited")
- )
+ val newAvdInfo = avdEditor(avdInfo)
avds += newAvdInfo
return newAvdInfo
}
@@ -154,6 +156,7 @@
avdInfo.dataFolderPath,
avdInfo.systemImage,
avdInfo.properties,
+ avdInfo.userSettings,
AvdInfo.AvdStatus.OK,
)
}
@@ -190,7 +193,8 @@
folderPath: Path = this.dataFolderPath,
systemImage: ISystemImage? = this.systemImage,
properties: Map<String, String> = this.properties,
+ userSettings: Map<String, String?>? = this.userSettings,
status: AvdInfo.AvdStatus = this.status,
-): AvdInfo = AvdInfo(name, iniFile, folderPath, systemImage, properties, status)
+): AvdInfo = AvdInfo(name, iniFile, folderPath, systemImage, properties, userSettings, status)
const val LAUNCH_EXCEPTION_MESSAGE = "launch_exception_message"
diff --git a/device-provisioner/src/test/com/android/sdklib/deviceprovisioner/LocalEmulatorProvisionerPluginTest.kt b/device-provisioner/src/test/com/android/sdklib/deviceprovisioner/LocalEmulatorProvisionerPluginTest.kt
index 51feffd..fe0f4cf 100644
--- a/device-provisioner/src/test/com/android/sdklib/deviceprovisioner/LocalEmulatorProvisionerPluginTest.kt
+++ b/device-provisioner/src/test/com/android/sdklib/deviceprovisioner/LocalEmulatorProvisionerPluginTest.kt
@@ -387,6 +387,47 @@
job.cancel()
}
+ // Some of the data for an AVD can be edited and updated while running.
+ // Check that it's fine to edit these properties while the AVD is offline or online.
+ @Test
+ fun editMetadata() = runBlockingWithTimeout {
+ val info = avdManager.makeAvdInfo(1, tag = SystemImageTags.GOOGLE_TV_TAG)
+ avdManager.createAvd(info)
+
+ val channel = Channel<DeviceState>()
+
+ yieldUntil { provisioner.devices.value.size == 1 }
+
+ val handle = provisioner.devices.value[0]
+ val job = launch { handle.stateFlow.collect { channel.send(it) } }
+
+ // Check if editing the AVD name works while offline.
+ val originalName = info.name
+ avdManager.avdEditor = { avdInfo: AvdInfo -> avdInfo.copy("New $originalName") }
+
+ handle.editAction?.edit()
+ channel.receiveUntilPassing { newState ->
+ assertThat((newState.properties as LocalEmulatorProperties).avdName)
+ .isEqualTo("New $originalName")
+ }
+
+ handle.activationAction?.activate()
+ channel.receiveUntilPassing { newState ->
+ assertThat(newState).isInstanceOf(Connected::class.java)
+ }
+
+ // Editing metadata while running shouldn't have problems.
+ avdManager.avdEditor = { avdInfo: AvdInfo -> avdInfo.copy("Final AVD name") }
+ handle.editAction?.edit()
+ channel.receiveUntilPassing { newState ->
+ assertThat((newState.properties as LocalEmulatorProperties).avdName)
+ .isEqualTo("Final AVD name")
+ assertThat(newState.error).isNull()
+ }
+
+ job.cancel()
+ }
+
/** Verify that the device updates the expected number of times. */
@Test
fun updateCount() = runBlockingWithTimeout {
diff --git a/sdklib/src/main/java/com/android/sdklib/internal/avd/AvdInfo.java b/sdklib/src/main/java/com/android/sdklib/internal/avd/AvdInfo.java
index 34c190e..9bd930e 100644
--- a/sdklib/src/main/java/com/android/sdklib/internal/avd/AvdInfo.java
+++ b/sdklib/src/main/java/com/android/sdklib/internal/avd/AvdInfo.java
@@ -73,6 +73,7 @@
@NonNull private final Path mIniFile;
@NonNull private final Path mFolderPath;
@NonNull private final ImmutableMap<String, String> mProperties;
+ @NonNull private final ImmutableMap<String, String> mUserSettings;
@NonNull private final AvdStatus mStatus;
@Nullable private final ISystemImage mSystemImage;
private final boolean mHasPlayStore;
@@ -93,8 +94,9 @@
@NonNull Path iniFile,
@NonNull Path folderPath,
@NonNull ISystemImage systemImage,
- @Nullable Map<String, String> properties) {
- this(name, iniFile, folderPath, systemImage, properties, AvdStatus.OK);
+ @Nullable Map<String, String> properties,
+ @Nullable Map<String, String> userSettings) {
+ this(name, iniFile, folderPath, systemImage, properties, userSettings, AvdStatus.OK);
}
/**
@@ -115,12 +117,15 @@
@NonNull Path folderPath,
@Nullable ISystemImage systemImage,
@Nullable Map<String, String> properties,
+ @Nullable Map<String, String> userSettings,
@NonNull AvdStatus status) {
mName = name;
mIniFile = iniFile;
mFolderPath = folderPath;
mSystemImage = systemImage;
mProperties = properties == null ? ImmutableMap.of() : ImmutableMap.copyOf(properties);
+ mUserSettings =
+ userSettings == null ? ImmutableMap.of() : ImmutableMap.copyOf(userSettings);
mStatus = status;
String psString = mProperties.get(AvdManager.AVD_INI_PLAYSTORE_ENABLED);
mHasPlayStore = "true".equalsIgnoreCase(psString) || "yes".equalsIgnoreCase(psString);
@@ -331,12 +336,13 @@
/** Helper method that returns the User Settings Path. */
@NonNull
- public Path getUserSettingsPath() {
- return mFolderPath.resolve(AvdManager.USER_SETTINGS_INI);
+ public static Path getUserSettingsPath(@NonNull Path dataFolder) {
+ return dataFolder.resolve(AvdManager.USER_SETTINGS_INI);
}
- public Map<String, String> parseUserSettingsFile(@Nullable ILogger logger) {
- PathFileWrapper settingsPath = new PathFileWrapper(getUserSettingsPath());
+ static Map<String, String> parseUserSettingsFile(
+ @NonNull Path dataFolder, @Nullable ILogger logger) {
+ PathFileWrapper settingsPath = new PathFileWrapper(getUserSettingsPath(dataFolder));
if (settingsPath.exists()) {
Map<String, String> parsedSettings = AvdManager.parseIniFile(settingsPath, logger);
if (parsedSettings != null) {
@@ -346,6 +352,11 @@
return new HashMap<>();
}
+ @NonNull
+ public Map<String, String> getUserSettings() {
+ return mUserSettings;
+ }
+
/** Returns the Config file for this AVD. */
@NonNull
public Path getConfigFile() {
@@ -411,6 +422,25 @@
return null;
}
+ public boolean isSameMetadata(@Nullable Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ AvdInfo avdInfo = (AvdInfo) o;
+ return mName.equals(avdInfo.mName) && mUserSettings.equals(avdInfo.mUserSettings);
+ }
+
+ @NonNull
+ public AvdInfo copyMetadata(@NonNull AvdInfo other) {
+ return new AvdInfo(
+ other.getName(),
+ getIniFile(),
+ getDataFolderPath(),
+ getSystemImage(),
+ getProperties(),
+ other.getUserSettings(),
+ getStatus());
+ }
+
@Override
public boolean equals(@Nullable Object o) {
if (this == o) return true;
@@ -421,6 +451,7 @@
&& mIniFile.equals(avdInfo.mIniFile)
&& mFolderPath.equals(avdInfo.mFolderPath)
&& mProperties.equals(avdInfo.mProperties)
+ && mUserSettings.equals(avdInfo.mUserSettings)
&& mStatus == avdInfo.mStatus
&& Objects.equals(mSystemImage, avdInfo.mSystemImage);
}
@@ -432,6 +463,7 @@
hashCode = 31 * hashCode + mIniFile.hashCode();
hashCode = 31 * hashCode + mFolderPath.hashCode();
hashCode = 31 * hashCode + mProperties.hashCode();
+ hashCode = 31 * hashCode + mUserSettings.hashCode();
hashCode = 31 * hashCode + mStatus.hashCode();
hashCode = 31 * hashCode + Objects.hashCode(mSystemImage);
hashCode = 31 * hashCode + Objects.hashCode(mHasPlayStore);
@@ -455,6 +487,9 @@
+ "mProperties = "
+ mProperties
+ separator
+ + "mUserSettings = "
+ + mUserSettings
+ + separator
+ "mStatus = "
+ mStatus
+ separator
diff --git a/sdklib/src/main/java/com/android/sdklib/internal/avd/AvdManager.java b/sdklib/src/main/java/com/android/sdklib/internal/avd/AvdManager.java
index f7e2a97..31ac78b 100644
--- a/sdklib/src/main/java/com/android/sdklib/internal/avd/AvdManager.java
+++ b/sdklib/src/main/java/com/android/sdklib/internal/avd/AvdManager.java
@@ -986,7 +986,8 @@
iniFile,
avdFolder,
oldAvdInfo,
- configValues);
+ configValues,
+ userSettings);
}
if ((removePrevious || editExisting) &&
@@ -1055,6 +1056,8 @@
// Modify the ID and display name in the new config.ini
Path configIni = destAvdFolder.resolve(CONFIG_INI);
Map<String, String> configVals = parseIniFile(new PathFileWrapper(configIni), mLog);
+ Map<String, String> userSettingsVals =
+ AvdInfo.parseUserSettingsFile(destAvdFolder, mLog);
configVals.put(AVD_INI_AVD_ID, newAvdName);
configVals.put(AVD_INI_DISPLAY_NAME, newAvdName);
writeIniFile(configIni, configVals, true);
@@ -1077,7 +1080,8 @@
newAvdName, destAvdFolder, false, systemImage.getAndroidVersion());
// Create an AVD object from these files
- return new AvdInfo(newAvdName, iniFile, destAvdFolder, systemImage, configVals);
+ return new AvdInfo(
+ newAvdName, iniFile, destAvdFolder, systemImage, configVals, userSettingsVals);
} catch (AndroidLocationsException | IOException e) {
mLog.warning("Exception while duplicating an AVD: %1$s", e);
return null;
@@ -1320,12 +1324,14 @@
}
// update AVD info
- AvdInfo info = new AvdInfo(
- avdInfo.getName(),
- avdInfo.getIniFile(),
- paramFolderPath,
- avdInfo.getSystemImage(),
- avdInfo.getProperties());
+ AvdInfo info =
+ new AvdInfo(
+ avdInfo.getName(),
+ avdInfo.getIniFile(),
+ paramFolderPath,
+ avdInfo.getSystemImage(),
+ avdInfo.getProperties(),
+ avdInfo.getUserSettings());
replaceAvd(avdInfo, info);
// update the ini file
@@ -1345,12 +1351,14 @@
}
// update AVD info
- AvdInfo info = new AvdInfo(
- newName,
- avdInfo.getIniFile(),
- avdInfo.getDataFolderPath(),
- avdInfo.getSystemImage(),
- avdInfo.getProperties());
+ AvdInfo info =
+ new AvdInfo(
+ newName,
+ avdInfo.getIniFile(),
+ avdInfo.getDataFolderPath(),
+ avdInfo.getSystemImage(),
+ avdInfo.getProperties(),
+ avdInfo.getUserSettings());
replaceAvd(avdInfo, info);
}
@@ -1489,7 +1497,7 @@
avdName = avdName.substring(0, avdName.length() - 4);
}
return new AvdInfo(
- avdName, iniPath, iniPath, null, null, AvdStatus.ERROR_CORRUPTED_INI);
+ avdName, iniPath, iniPath, null, null, null, AvdStatus.ERROR_CORRUPTED_INI);
}
PathFileWrapper configIniFile;
@@ -1623,7 +1631,10 @@
}
}
- AvdInfo info = new AvdInfo(name, iniPath, avdPath, sysImage, properties, status);
+ Map<String, String> userSettings = AvdInfo.parseUserSettingsFile(avdPath, mLog);
+
+ AvdInfo info =
+ new AvdInfo(name, iniPath, avdPath, sysImage, properties, userSettings, status);
if (updateHashV2) {
try {
@@ -1853,13 +1864,16 @@
// finally create a new AvdInfo for this unbroken avd and add it to the list.
// instead of creating the AvdInfo object directly we reparse it, to detect other possible
// errors
- // FIXME: We may want to create this AvdInfo by reparsing the AVD instead. This could detect other errors.
- AvdInfo newAvd = new AvdInfo(
- avd.getName(),
- avd.getIniFile(),
- avd.getDataFolderPath(),
- avd.getSystemImage(),
- newProperties);
+ // FIXME: We may want to create this AvdInfo by reparsing the AVD instead. This could detect
+ // other errors.
+ AvdInfo newAvd =
+ new AvdInfo(
+ avd.getName(),
+ avd.getIniFile(),
+ avd.getDataFolderPath(),
+ avd.getSystemImage(),
+ newProperties,
+ avd.getUserSettings());
replaceAvd(avd, newAvd);
@@ -2290,11 +2304,13 @@
@NonNull Path iniFile,
@NonNull Path avdFolder,
@Nullable AvdInfo oldAvdInfo,
- @Nullable Map<String, String> values)
+ @Nullable Map<String, String> values,
+ @Nullable Map<String, String> userSettings)
throws AvdMgrException {
// create the AvdInfo object, and add it to the list
- AvdInfo theAvdInfo = new AvdInfo(avdName, iniFile, avdFolder, systemImage, values);
+ AvdInfo theAvdInfo =
+ new AvdInfo(avdName, iniFile, avdFolder, systemImage, values, userSettings);
synchronized (mAllAvdList) {
if (oldAvdInfo != null && (removePrevious || editExisting)) {
