Skip to content

Commit

Permalink
Minor clarity cleanups for SpawnStrategyRegistry's Builder
Browse files Browse the repository at this point in the history
Remove unused registerSpawnStrategy(..., List) - easier to figure out how
strategies are registered if it's only done via one method.

Include "dynamic" in the naming of the mnemonic->strategy maps for dynamic
execution, making it more clear that these just apply to dynamic execution (note
the main-class variables are already named this way). This should also help
casual passerby differentiate these from remoteLocalFallbackStrategy.

Finally, logically separate vars by "stuff that's from registerting strategies"
and "stuff that configures how strategies are chosen".

PiperOrigin-RevId: 460279530
Change-Id: I3729aef95b17324bc3c7b6b3dc46017eac08c849
  • Loading branch information
michajlo authored and copybara-github committed Jul 11, 2022
1 parent 943e8cf commit 9275daa
Showing 1 changed file with 14 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -250,18 +250,21 @@ public static Builder builder() {
*/
public static final class Builder {

private final HashMap<String, SpawnStrategy> identifierToStrategy = new HashMap<>();
private final ArrayList<SpawnStrategy> strategiesInRegistrationOrder = new ArrayList<>();

private ImmutableList<String> explicitDefaultStrategies = ImmutableList.of();

// TODO(schmitt): Using a list and autovalue so as to be able to reverse order while legacy sort
// is supported. Can be converted to same as mnemonics once legacy behavior is removed.
private final List<FilterAndIdentifiers> filterAndIdentifiers = new ArrayList<>();
private final HashMap<String, SpawnStrategy> identifierToStrategy = new HashMap<>();
private final ArrayList<SpawnStrategy> strategiesInRegistrationOrder = new ArrayList<>();

// Using List values here rather than multimaps as there is no need for the latter's
// functionality: The values are always replaced as a whole, no adding/creation required.
private final HashMap<String, List<String>> mnemonicToIdentifiers = new HashMap<>();
private final HashMap<String, List<String>> mnemonicToRemoteIdentifiers = new HashMap<>();
private final HashMap<String, List<String>> mnemonicToLocalIdentifiers = new HashMap<>();
private final HashMap<String, List<String>> mnemonicToRemoteDynamicIdentifiers =
new HashMap<>();
private final HashMap<String, List<String>> mnemonicToLocalDynamicIdentifiers = new HashMap<>();

@Nullable private String remoteLocalFallbackStrategyIdentifier;

/**
Expand Down Expand Up @@ -306,20 +309,16 @@ public Builder addMnemonicFilter(String mnemonic, List<String> identifiers) {
* so registered will take precedence.
*/
@CanIgnoreReturnValue
public Builder registerStrategy(SpawnStrategy strategy, List<String> commandlineIdentifiers) {
public Builder registerStrategy(SpawnStrategy strategy, String... commandlineIdentifiers) {
Preconditions.checkArgument(
commandlineIdentifiers.size() >= 1, "At least one commandLineIdentifier must be given");
commandlineIdentifiers.length >= 1, "At least one commandLineIdentifier must be given");
for (String identifier : commandlineIdentifiers) {
identifierToStrategy.put(identifier, strategy);
}
strategiesInRegistrationOrder.add(strategy);
return this;
}

public Builder registerStrategy(SpawnStrategy strategy, String... commandlineIdentifiers) {
return registerStrategy(strategy, ImmutableList.copyOf(commandlineIdentifiers));
}

/**
* Explicitly sets the identifiers of default strategies to use if a spawn matches no filters.
*
Expand Down Expand Up @@ -357,7 +356,7 @@ public Builder resetDefaultStrategies() {
*/
@CanIgnoreReturnValue
public Builder addDynamicRemoteStrategies(Map<String, List<String>> strategies) {
mnemonicToRemoteIdentifiers.putAll(strategies);
mnemonicToRemoteDynamicIdentifiers.putAll(strategies);
return this;
}

Expand All @@ -371,7 +370,7 @@ public Builder addDynamicRemoteStrategies(Map<String, List<String>> strategies)
*/
@CanIgnoreReturnValue
public Builder addDynamicLocalStrategies(Map<String, List<String>> strategies) {
mnemonicToLocalIdentifiers.putAll(strategies);
mnemonicToLocalDynamicIdentifiers.putAll(strategies);
return this;
}

Expand Down Expand Up @@ -417,15 +416,15 @@ public SpawnStrategyRegistry build() throws AbruptExitException {

ImmutableListMultimap.Builder<String, SandboxedSpawnStrategy> mnemonicToLocalStrategies =
new ImmutableListMultimap.Builder<>();
for (Map.Entry<String, List<String>> entry : mnemonicToLocalIdentifiers.entrySet()) {
for (Map.Entry<String, List<String>> entry : mnemonicToLocalDynamicIdentifiers.entrySet()) {
mnemonicToLocalStrategies.putAll(
entry.getKey(),
toSandboxedStrategies(entry.getValue(), "local mnemonic " + entry.getKey()));
}

ImmutableListMultimap.Builder<String, SandboxedSpawnStrategy> mnemonicToRemoteStrategies =
new ImmutableListMultimap.Builder<>();
for (Map.Entry<String, List<String>> entry : mnemonicToRemoteIdentifiers.entrySet()) {
for (Map.Entry<String, List<String>> entry : mnemonicToRemoteDynamicIdentifiers.entrySet()) {
mnemonicToRemoteStrategies.putAll(
entry.getKey(),
toSandboxedStrategies(entry.getValue(), "remote mnemonic " + entry.getKey()));
Expand Down

0 comments on commit 9275daa

Please sign in to comment.