Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
*/
package com.salesforce.androidsdk.config;

import static java.lang.String.format;
import static java.util.Locale.US;

import android.content.Context;
import android.content.SharedPreferences;
import android.content.SharedPreferences.Editor;
Expand Down Expand Up @@ -130,7 +133,7 @@ public LoginServer getSelectedLoginServer() {
if (name != null && url != null) {
LoginServer server = new LoginServer(name, url, isCustom);

// Only notify livedata consumers if the value has changed.
// Only notify live data consumers if the value has changed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: I think LiveData is probably the most correct. Google never writes it as two words.

if (!server.equals(selectedServer.getValue())) {
selectedServer.postValue(server);
}
Expand Down Expand Up @@ -323,6 +326,94 @@ public List<LoginServer> getLoginServersFromPreferences() {
return getLoginServersFromPreferences(settings);
}

/**
* Reorders a custom login server in the list of login servers.
*
* @param originalIndex The original index of the custom login server. If this is not the index
* of a custom login server, this method will do nothing
* @param updatedIndex The new index of the custom login server. This must be after the last
* non-custom login server and within the updatable bounds of the list. If
* it is not it will be automatically corrected
*/
@SuppressWarnings("unused")
public void reorderCustomLoginServer(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation comment should provide adequate guidance, particularly where the boundary of non-custom and custom login server indexes is guarded.

@brandonpage brandonpage Feb 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the interface they wanted? I was envisioning something like:
public void reorderLoginServers(List<LoginServer> reorderedList)

Where they just gave us the list of servers in the order they wanted.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this only reorders custom login servers. Is that what they want?

final int originalIndex,
int updatedIndex
) {
// Get the login server at the original index.
final List<LoginServer> loginServers = getLoginServers();
final LoginServer originalLoginServer = loginServers.get(originalIndex);

// Guard against reordering a non-custom login server.
if (!originalLoginServer.isCustom) {
return;
}

// Determine the last non-custom login server index.
final List<LoginServer> servers = getLoginServers();
int firstCustomLoginServerIndex = -1;
for (int i = servers.size() - 1; i >= 0; i--) {
if (servers.get(i).isCustom) {
firstCustomLoginServerIndex = i;
}
}

// Adjust the re-ordered custom login server index to be within bounds.
if (updatedIndex <= firstCustomLoginServerIndex) {
updatedIndex = firstCustomLoginServerIndex;
} else if (updatedIndex >= servers.size()) {
updatedIndex = servers.size() - 1;
}

// Update the login server list.
loginServers.remove(originalIndex);
loginServers.add(updatedIndex, originalLoginServer);

// Edit each login server indexed after the updated index.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "re-indexing" logic is very similar to what's already in remove login server.

final Editor editor = settings.edit();
for (int i = updatedIndex; i < loginServers.size(); i++) {
final LoginServer loginServer = loginServers.get(i);
editor.remove(format(US, SERVER_NAME, i))
.remove(format(US, SERVER_URL, i))
.remove(format(US, IS_CUSTOM, i))
.putString(format(US, SERVER_NAME, i), loginServer.name)
.putString(format(US, SERVER_URL, i), loginServer.url)
.putBoolean(format(US, IS_CUSTOM, i), loginServer.isCustom);
}
editor.apply();
}

/**
* Replaces one custom login server with another.
*
* @param originalCustomLoginServer The original custom login server. If this is not a custom
* login server or doesn't match an existing login server this
* method will do nothing.
* @param updatedCustomLoginServer The updated custom login server. If this is not a custom
* login server this method will do nothing.
*/
@SuppressWarnings("unused")
public void replaceCustomLoginServer(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation comment here should guide callers around legal parameters.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it not possible to add an update(name, url) method to LoginServer itself?

final LoginServer originalCustomLoginServer,
final LoginServer updatedCustomLoginServer
) {
// Guard against replacing a non-custom login server.
if (!originalCustomLoginServer.isCustom || !updatedCustomLoginServer.isCustom) {
return;
}

final int originalIndex = getLoginServers().indexOf(originalCustomLoginServer);

// Guard against an original login server that doesn't exist.
if (originalIndex == -1) {
return;
}

removeServer(originalCustomLoginServer);
addCustomLoginServer(updatedCustomLoginServer.name, updatedCustomLoginServer.url);
reorderCustomLoginServer(getLoginServers().size() - 1, originalIndex);
}

/**
* Returns production and sandbox as the login servers
* (only called when servers.xml is missing).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
import android.content.Context;

import androidx.arch.core.executor.testing.InstantTaskExecutorRule;
import androidx.test.platform.app.InstrumentationRegistry;
import androidx.test.filters.SmallTest;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import androidx.test.filters.SmallTest;
import androidx.test.platform.app.InstrumentationRegistry;

import com.salesforce.androidsdk.TestForceApp;
import com.salesforce.androidsdk.app.SalesforceSDKManager;
Expand Down Expand Up @@ -347,6 +347,126 @@ public void testAddingDuplicateServers() {
loginServerManager.getLoginServers().size());
}

/**
* Test both replace and re-order custom login server.
*/
@Test
public void testReplaceAndReOrderCustomLoginServer() {

// Test data.
final String originalName = "ORIGINAL_CUSTOM_LOGIN_SERVER_FOR_REPLACEMENT_TEST";
final String originalUrl = "https://original.example.com";
final LoginServer originalCustomLoginServer = new LoginServer(
originalName,
originalUrl,
true
);
final String otherName = "OTHER_CUSTOM_LOGIN_SERVER_FOR_REPLACEMENT_TEST";
final String otherUrl = "https://other.example.com";
final LoginServer otherCustomLoginServer = new LoginServer(
otherName,
otherUrl,
true
);
final String updatedName = "UPDATED_CUSTOM_LOGIN_SERVER_FOR_REPLACEMENT_TEST";
final String updatedUrl = "https://updated.example.com";
final LoginServer updatedCustomLoginServer = new LoginServer(
updatedName,
updatedUrl,
true
);
final String nonCustomName = "NON_CUSTOM_LOGIN_SERVER_FOR_REPLACEMENT_TEST";
final String nonCustomUrl = "https://non.custom.example.com";
final LoginServer nonCustomLoginServer = new LoginServer(
nonCustomName,
nonCustomUrl,
false
);

// Verify the original and other custom login servers are not present.
Assert.assertFalse(loginServerManager.getLoginServers().contains(originalCustomLoginServer));
Assert.assertFalse(loginServerManager.getLoginServers().contains(otherCustomLoginServer));


// Add the original and other custom login server.
loginServerManager.addCustomLoginServer(originalName, originalUrl);
loginServerManager.addCustomLoginServer(otherName, otherUrl);

// Verify the original and other custom login servers were added.
Assert.assertEquals(originalCustomLoginServer, loginServerManager.getLoginServers().get(loginServerManager.getLoginServers().size() - 2));
Assert.assertEquals(otherCustomLoginServer, loginServerManager.getLoginServers().get(loginServerManager.getLoginServers().size() - 1));


// Prepare for negative tests.
final LoginServer production = new LoginServer("Production", "https://login.salesforce.com", false);
final LoginServer productionMismatch = new LoginServer("Production?", "https://login.salesforce.com", true);
final LoginServer productionReplacement = new LoginServer("Production Replaced", "https://login.salesforce.com", false);
final LoginServer productionReplacementMismatch = new LoginServer("Production Replaced?", "https://login.salesforce.com", true);

// Attempt the prohibited replacement of a non-custom login server where the original matches.
loginServerManager.replaceCustomLoginServer(production, productionReplacement);
Assert.assertTrue(loginServerManager.getLoginServers().contains(production));
Assert.assertFalse(loginServerManager.getLoginServers().contains(productionReplacement));


// Attempt the prohibited replacement of a non-custom login server where the original doesn't exit.
loginServerManager.replaceCustomLoginServer(productionMismatch, productionReplacementMismatch);
Assert.assertTrue(loginServerManager.getLoginServers().contains(production));
Assert.assertFalse(loginServerManager.getLoginServers().contains(productionReplacement));


// Attempt the prohibited reordering of a non-custom login server.
loginServerManager.reorderCustomLoginServer(0, 1);
Assert.assertEquals(loginServerManager.getLoginServers().get(0), production);


// Replace the original custom login server with a non-custom server.
loginServerManager.replaceCustomLoginServer(originalCustomLoginServer, nonCustomLoginServer);

// Verify the original and other custom login servers weren't changed.
Assert.assertFalse(loginServerManager.getLoginServers().contains(nonCustomLoginServer));
Assert.assertEquals(originalCustomLoginServer, loginServerManager.getLoginServers().get(loginServerManager.getLoginServers().size() - 2));
Assert.assertEquals(otherCustomLoginServer, loginServerManager.getLoginServers().get(loginServerManager.getLoginServers().size() - 1));


// Replace the original custom login server.
loginServerManager.replaceCustomLoginServer(originalCustomLoginServer, updatedCustomLoginServer);

// Verify the original custom login server is not present.
Assert.assertFalse(loginServerManager.getLoginServers().contains(originalCustomLoginServer));

// Verify the updated and other custom login servers are present.
Assert.assertEquals(updatedCustomLoginServer, loginServerManager.getLoginServers().get(loginServerManager.getLoginServers().size() - 2));
Assert.assertEquals(otherCustomLoginServer, loginServerManager.getLoginServers().get(loginServerManager.getLoginServers().size() - 1));

// Attempt to move the updated custom login server above the non-custom login servers.
loginServerManager.reorderCustomLoginServer(loginServerManager.getLoginServers().indexOf(updatedCustomLoginServer), 0);

// Verify the updated custom login server is actually immediately following the last non-custom login server.
final List<LoginServer> loginServers = loginServerManager.getLoginServers();
int lastNonCustomIndex = -1;
for (int i = 0; i < loginServers.size(); i++) {
final LoginServer loginServer = loginServers.get(i);
if (!loginServer.isCustom) {
lastNonCustomIndex = i;
}
}
Assert.assertEquals(loginServers.get(lastNonCustomIndex + 1), updatedCustomLoginServer);


// Attempt to move the updated custom login server one greater than the upper bounds of the login servers list.
loginServerManager.reorderCustomLoginServer(loginServerManager.getLoginServers().indexOf(updatedCustomLoginServer), loginServerManager.getLoginServers().size());

// Attempt to move the updated custom login server more than one greater than the upper bounds of the login servers list.
loginServerManager.reorderCustomLoginServer(loginServerManager.getLoginServers().indexOf(updatedCustomLoginServer), loginServerManager.getLoginServers().size() + 1);

// Attempt to move the updated custom login server more than one less than the upper bounds of the login servers list.
loginServerManager.reorderCustomLoginServer(loginServerManager.getLoginServers().indexOf(updatedCustomLoginServer), loginServerManager.getLoginServers().size() - 1);

// Verify the updated custom login server is now the last login server in the list.
Assert.assertEquals(loginServerManager.getLoginServers().getLast(), updatedCustomLoginServer);
}

private void assertProduction(LoginServer server) {
Assert.assertEquals("Expected production's name", "Production", server.name);
Assert.assertEquals("Expected production's url", PRODUCTION_URL, server.url);
Expand Down