Skip to content

Commit

Permalink
config: fix duplicating rsprofile
Browse files Browse the repository at this point in the history
Even though the web service v2 to v3 migration names the $rsprofile
correctly, profiles created since the addition of the v3 api were never
named.

Commonly, this caused a duplication of profile -1. During profile sync,
the client would apply the remote profile name "" to $rsprofile, and
then later recreate $rsprofile due to no profile with the name existing.

The config manager rsProfile was then this newly created $rsProfile, so
the disk storage of the profile has been in the correct location.

The "" profile then shows up in the profile panel, and deleting it would
then delete both it and the $rsprofile profile - causing the config
manager to recreate it due to having lost its active profile.

This detects and removes duplicate profiles, and corrects the name of
the rsprofile profile.

I've also included the profile name in the config patch, so edits to the
profile and new rsprofiles always have the correct name.

This also fixes trying to sync newly created profiles with no settings, which
previously 400d due to posting a null patch. This caused the profile to
not really be synced and be considered "lost" at the next profile sync.
  • Loading branch information
Adam- committed Aug 17, 2023
1 parent 45d0aed commit 9812a0c
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ public void onSessionOpen(SessionOpen sessionOpen)
// instead of overwritten. After a login send a PATCH for the offline $rsprofile to merge it with the
// remote $rsprofile so that when $rsprofile is synced later it doesn't overwrite and lose the local
// $rsprofile settings.
ConfigPatch patch = buildConfigPatch(rsProfileConfigProfile.get());
ConfigPatch patch = buildConfigPatch(rsProfile.getName(), rsProfileConfigProfile.get());
configClient.patch(patch, rsProfile.getId());
log.debug("patched remote {}", RSPROFILE_NAME);
}
Expand Down Expand Up @@ -318,12 +318,8 @@ public void toggleSync(ConfigProfile profile, boolean sync)
// sync the entire profile from disk
File from = ProfileManager.profileConfigFile(profile);
ConfigData data = new ConfigData(from);
ConfigPatch patch = buildConfigPatch(data.get());

long id = profile.getId();
String name = profile.getName();

configClient.patch(patch, profile.getId()).thenRun(() -> configClient.rename(id, name));
ConfigPatch patch = buildConfigPatch(null, data.get());
configClient.patch(patch, profile.getId());
}
else
{
Expand Down Expand Up @@ -462,6 +458,37 @@ public void importAndMigrate(ProfileManager.Lock lock, File from, ConfigProfile
log.info("Finished importing {} keys", keys);
}

private static void removeDuplicateProfiles(ProfileManager.Lock lock)
{
var seen = new HashMap<Long, ConfigProfile>();
for (var it = lock.getProfiles().iterator(); it.hasNext(); )
{
var profile = it.next();
if (seen.containsKey(profile.getId()))
{
var existing = seen.get(profile.getId());
log.warn("Duplicate profiles detected: {} and {}. Removing the latter.",
existing, profile);
it.remove();
lock.dirty();
continue;
}

seen.put(profile.getId(), profile);
}
}

private static void fixRsProfileName(ProfileManager.Lock lock)
{
var rsProfile = lock.findProfile(RSPROFILE_ID);
if (rsProfile != null && !rsProfile.getName().equals(RSPROFILE_NAME))
{
log.warn("renaming {} to {}", rsProfile, RSPROFILE_NAME);
rsProfile.setName(RSPROFILE_NAME);
lock.dirty();
}
}

public void load()
{
AccountSession session = sessionManager.getAccountSession();
Expand All @@ -488,12 +515,17 @@ public void load()

try (ProfileManager.Lock lock = profileManager.lock())
{
removeDuplicateProfiles(lock);
fixRsProfileName(lock);

ConfigProfile profile = null, rsProfile = null;

for (ConfigProfile p : lock.getProfiles())
{
if (p.isInternal())
{
log.debug("Profile '{}' (sync: {}, active: {}, internal)", p.getName(), p.isSync(), p.isActive());

if (p.getName().equals(RSPROFILE_NAME))
{
rsProfile = p;
Expand Down Expand Up @@ -1278,7 +1310,7 @@ private void saveConfiguration(ProfileManager.Lock lock, ConfigProfile profile,
{
try
{
ConfigPatchResult patchResult = configClient.patch(buildConfigPatch(patch), profile.getId()).get();
ConfigPatchResult patchResult = configClient.patch(buildConfigPatch(profile.isInternal() ? profile.getName() : null, patch), profile.getId()).get();
if (patchResult == null)
{
profile.setRev(-1L);
Expand Down Expand Up @@ -1314,14 +1346,12 @@ private void saveConfiguration(ProfileManager.Lock lock, ConfigProfile profile,
data.patch(patch);
}

private static ConfigPatch buildConfigPatch(Map<String, String> patchChanges)
private static ConfigPatch buildConfigPatch(@Nullable String profileName, Map<String, String> patchChanges)
{
if (patchChanges.isEmpty())
{
return null;
}

ConfigPatch patch = new ConfigPatch();
// Note profileName is only used for internal profiles which don't get renamed, to avoid
// clients fighting over profiles names.
patch.setProfileName(profileName);
for (Map.Entry<String, String> entry : patchChanges.entrySet())
{
final String key = entry.getKey(), value = entry.getValue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ public List<ConfigProfile> getProfiles()

public ConfigProfile createProfile(String name, long id)
{
if (findProfile(id) != null)
{
throw new IllegalArgumentException("profile " + id + " already exists");
}

ConfigProfile profile = new ConfigProfile(id);
profile.setName(name);
profile.setSync(false);
Expand Down

0 comments on commit 9812a0c

Please sign in to comment.