Skip to content

Commit

Permalink
Simplify RefreshOperation protocol
Browse files Browse the repository at this point in the history
- Merge `setQueryOutput(output)` and `createBlazeProject()` into one operation
`createBlazeProject(output)`.
- Make refresh operation instances immutable.
- Remove precautions from comments and JavaDocs.

This is a step of a bigger refactoring aimed a moving `BlazeProjectSnapshot`
manipulation out of refresh operations to a separate component tha can also
modify the project structure based on the set of built dependencies.

PiperOrigin-RevId: 569179242
  • Loading branch information
Googler authored and copybara-github committed Oct 2, 2023
1 parent e0f851c commit 6925957
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package com.google.idea.blaze.base.qsync;


import com.google.common.annotations.VisibleForTesting;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.Uninterruptibles;
Expand Down Expand Up @@ -79,9 +78,7 @@ public BlazeProjectSnapshot fullQuery(ProjectDefinition projectDef, BlazeContext
RefreshOperation fullQuery = projectRefresher.startFullUpdate(context, projectDef, vcsState);

QuerySpec querySpec = fullQuery.getQuerySpec().get();
fullQuery.setQueryOutput(queryRunner.runQuery(querySpec, context));

return fullQuery.createBlazeProject();
return fullQuery.createBlazeProject(queryRunner.runQuery(querySpec, context));
}

private Optional<VcsState> getVcsState(BlazeContext context) {
Expand Down Expand Up @@ -130,11 +127,12 @@ public BlazeProjectSnapshot update(
projectRefresher.startPartialRefresh(context, previousState, vcsState, currentProjectDef);

Optional<QuerySpec> spec = refresh.getQuerySpec();
QuerySummary querySummary;
if (spec.isPresent()) {
refresh.setQueryOutput(queryRunner.runQuery(spec.get(), context));
querySummary = queryRunner.runQuery(spec.get(), context);
} else {
refresh.setQueryOutput(QuerySummary.EMPTY);
querySummary = QuerySummary.EMPTY;
}
return refresh.createBlazeProject();
return refresh.createBlazeProject(querySummary);
}
}
20 changes: 9 additions & 11 deletions querysync/java/com/google/idea/blaze/qsync/FullProjectUpdate.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,9 @@ public class FullProjectUpdate implements RefreshOperation {
private final Path effectiveWorkspaceRoot;
private final BlazeQueryParser.Factory queryParserFactory;
private final ProjectDefinition projectDefinition;
private final Optional<VcsState> vcsState;
private final GraphToProjectConverter graphToProjectConverter;

private final PostQuerySyncData.Builder result;

public FullProjectUpdate(
Context<?> context,
ListeningExecutorService executor,
Expand All @@ -57,9 +56,8 @@ public FullProjectUpdate(
ImmutableSet<String> handledRuleKinds) {
this.context = context;
this.effectiveWorkspaceRoot = effectiveWorkspaceRoot;
this.result =
PostQuerySyncData.builder().setProjectDefinition(definition).setVcsState(vcsState);
this.projectDefinition = definition;
this.vcsState = vcsState;
this.queryParserFactory = new BlazeQueryParser.Factory(context, handledRuleKinds);
this.graphToProjectConverter =
new GraphToProjectConverter(
Expand All @@ -73,13 +71,13 @@ public Optional<QuerySpec> getQuerySpec() throws IOException {
}

@Override
public void setQueryOutput(QuerySummary output) {
result.setQuerySummary(output);
}

@Override
public BlazeProjectSnapshot createBlazeProject() throws BuildException {
PostQuerySyncData newData = result.build();
public BlazeProjectSnapshot createBlazeProject(QuerySummary output) throws BuildException {
PostQuerySyncData newData =
PostQuerySyncData.builder()
.setProjectDefinition(projectDefinition)
.setVcsState(vcsState)
.setQuerySummary(output)
.build();
BuildGraphData graph = queryParserFactory.newParser(newData.querySummary()).parse();
ProjectProto.Project project = graphToProjectConverter.createProject(graph);
return BlazeProjectSnapshot.builder().queryData(newData).graph(graph).project(project).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,7 @@ public Optional<QuerySpec> getQuerySpec() {
}

@Override
public void setQueryOutput(QuerySummary output) {}

@Override
public BlazeProjectSnapshot createBlazeProject() {
public BlazeProjectSnapshot createBlazeProject(QuerySummary output) {
return latestProjectSnapshotSupplier.get();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ class PartialProjectRefresh implements RefreshOperation {
private final PostQuerySyncData.Builder newState;
@VisibleForTesting final ImmutableSet<Path> modifiedPackages;
@VisibleForTesting final ImmutableSet<Path> deletedPackages;
private QuerySummary partialQuery;

PartialProjectRefresh(
Context context,
Expand Down Expand Up @@ -102,14 +101,9 @@ public Optional<QuerySpec> getQuerySpec() {
}

@Override
public void setQueryOutput(QuerySummary output) {
this.partialQuery = output;
}

@Override
public BlazeProjectSnapshot createBlazeProject() throws BuildException {
public BlazeProjectSnapshot createBlazeProject(QuerySummary partialQuery) throws BuildException {
Preconditions.checkNotNull(partialQuery, "queryOutput");
QuerySummary effectiveQuery = applyDelta();
QuerySummary effectiveQuery = applyDelta(partialQuery);
PostQuerySyncData postQuerySyncData = newState.setQuerySummary(effectiveQuery).build();
BuildGraphData graph = queryParserFactory.newParser(effectiveQuery).parse();
ProjectProto.Project project = graphToProjectConverter.createProject(graph);
Expand All @@ -125,7 +119,7 @@ public BlazeProjectSnapshot createBlazeProject() throws BuildException {
* partial query, and any deleted packages.
*/
@VisibleForTesting
QuerySummary applyDelta() {
QuerySummary applyDelta(QuerySummary partialQuery) {
// copy all unaffected rules / source files to result:
Map<Label, SourceFile> newSourceFiles = Maps.newHashMap();
for (Map.Entry<Label, SourceFile> sfEntry :
Expand Down
14 changes: 4 additions & 10 deletions querysync/java/com/google/idea/blaze/qsync/RefreshOperation.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,15 @@
* <ol>
* <li>Acquire an implementation, e.g. from {@link ProjectRefresher}
* <li>Run a {@code query} invocation based on the spec from {@link #getQuerySpec()}
* <li>Pass the results of that query to {@link #setQueryOutput(QuerySummary)}
* <li>Call {@link #createBlazeProject()} to get the updated project snapshot.
* <li>To get the updated project snapshot pass the results of that query to {@link
* #createBlazeProject(QuerySummary)}.
* </ol>
*/
public interface RefreshOperation {

/** Returns the spec of the query to be run for this strategy. */
Optional<QuerySpec> getQuerySpec() throws IOException;

/** Passes the output from the query specified by {@link #getQuerySpec()}. */
void setQueryOutput(QuerySummary output);

/**
* Creates the new project snapshot. Must only be called after {@link
* #setQueryOutput(QuerySummary)}.
*/
BlazeProjectSnapshot createBlazeProject() throws BuildException;
/** Creates the new project snapshot. */
BlazeProjectSnapshot createBlazeProject(QuerySummary output) throws BuildException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,7 @@ public void testApplyDelta_replacePackage() {
/* modifiedPackages= */ ImmutableSet.of(Path.of("my/build/package1")),
ImmutableSet.of(),
ImmutableSet.of());
queryStrategy.setQueryOutput(delta);
QuerySummary applied = queryStrategy.applyDelta();
QuerySummary applied = queryStrategy.applyDelta(delta);
assertThat(applied.getRulesMap().keySet())
.containsExactly(
Label.of("//my/build/package1:newrule"), Label.of("//my/build/package2:rule"));
Expand Down Expand Up @@ -179,8 +178,7 @@ public void testApplyDelta_deletePackage() {
/* deletedPackages= */ ImmutableSet.of(Path.of("my/build/package1")),
ImmutableSet.of());
assertThat(queryStrategy.getQuerySpec()).isEmpty();
queryStrategy.setQueryOutput(QuerySummary.EMPTY);
QuerySummary applied = queryStrategy.applyDelta();
QuerySummary applied = queryStrategy.applyDelta(QuerySummary.EMPTY);
assertThat(applied.getRulesMap().keySet())
.containsExactly(Label.of("//my/build/package2:rule"));
assertThat(applied.getSourceFilesMap().keySet())
Expand Down Expand Up @@ -244,8 +242,7 @@ public void testDelta_addPackage() {
/* modifiedPackages= */ ImmutableSet.of(Path.of("my/build/package2")),
ImmutableSet.of(),
ImmutableSet.of());
queryStrategy.setQueryOutput(delta);
QuerySummary applied = queryStrategy.applyDelta();
QuerySummary applied = queryStrategy.applyDelta(delta);
assertThat(applied.getRulesMap().keySet())
.containsExactly(
Label.of("//my/build/package1:rule"), Label.of("//my/build/package2:rule"));
Expand Down Expand Up @@ -279,8 +276,7 @@ public void testDelta_packagesWithErrors() {
/* modifiedPackages= */ ImmutableSet.of(Path.of("my/build/package")),
ImmutableSet.of(),
ImmutableSet.of());
queryStrategy.setQueryOutput(delta);
QuerySummary applied = queryStrategy.applyDelta();
QuerySummary applied = queryStrategy.applyDelta(delta);
assertThat(applied.getPackagesWithErrors()).containsExactly(Path.of("my/build/package"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public void testStartPartialRefresh_vcsSnapshotUnchanged_existingProjectSnapshot
project.vcsState(),
project.projectDefinition());
assertThat(update).isInstanceOf(NoopProjectRefresh.class);
assertThat(update.createBlazeProject()).isSameInstanceAs(existingProject);
assertThat(update.createBlazeProject(QuerySummary.EMPTY)).isSameInstanceAs(existingProject);
}

@Test
Expand Down

0 comments on commit 6925957

Please sign in to comment.