Replace generic other_attributes with specific fields in Query.Summar…

…y proto. The generic map forms an informal API, whereas making it explicit is safer and easier to reason about. PiperOrigin-RevId: 555103071
bazelbuild · Aug 9, 2023 · 0ebdb0e · 0ebdb0e
1 parent 5921f52
commit 0ebdb0e
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 20 deletions.
diff --git a/querysync/java/com/google/idea/blaze/qsync/BlazeQueryParser.java b/querysync/java/com/google/idea/blaze/qsync/BlazeQueryParser.java
@@ -115,17 +115,14 @@ public BuildGraphData parse(QuerySummary query) {
 
       BuildTarget.Builder buildTarget =
           BuildTarget.builder().setLabel(ruleEntry.getKey()).setKind(ruleClass);
-      if (ruleEntry.getValue().containsOtherAttributes("test_app")) {
-        buildTarget.setTestApp(
-            Label.of(ruleEntry.getValue().getOtherAttributesOrThrow("test_app")));
+      if (!ruleEntry.getValue().getTestApp().isEmpty()) {
+        buildTarget.setTestApp(Label.of(ruleEntry.getValue().getTestApp()));
       }
-      if (ruleEntry.getValue().containsOtherAttributes("instruments")) {
-        buildTarget.setInstruments(
-            Label.of(ruleEntry.getValue().getOtherAttributesOrThrow("instruments")));
+      if (!ruleEntry.getValue().getInstruments().isEmpty()) {
+        buildTarget.setInstruments(Label.of(ruleEntry.getValue().getInstruments()));
       }
-      if (ruleEntry.getValue().containsOtherAttributes("custom_package")) {
-        buildTarget.setCustomPackage(
-            ruleEntry.getValue().getOtherAttributesOrThrow("custom_package"));
+      if (!ruleEntry.getValue().getCustomPackage().isEmpty()) {
+        buildTarget.setCustomPackage(ruleEntry.getValue().getCustomPackage());
       }
       graphBuilder.targetMapBuilder().put(ruleEntry.getKey(), buildTarget.build());
 

diff --git a/querysync/java/com/google/idea/blaze/qsync/query/QuerySummary.java b/querysync/java/com/google/idea/blaze/qsync/query/QuerySummary.java
@@ -71,15 +71,11 @@ public abstract class QuerySummary {
    * <p>Whenever changing the logic in this class such that the Query.Summary proto contents will be
    * different for the same input, this version should be incremented.
    */
-  @VisibleForTesting public static final int PROTO_VERSION = 2;
+  @VisibleForTesting public static final int PROTO_VERSION = 3;
 
   public static final QuerySummary EMPTY =
       create(Query.Summary.newBuilder().setVersion(PROTO_VERSION).build());
 
-  // Other rule attributes needed by query sync. Only supports attributes with single-string values
-  private static final ImmutableSet<String> OTHER_ATTRIBUTES =
-      ImmutableSet.of("test_app", "instruments", "custom_package");
-
   // Compile-time dependency attributes
   private static final ImmutableSet<String> DEPENDENCY_ATTRIBUTES =
       ImmutableSet.of(
@@ -161,10 +157,12 @@ public static QuerySummary create(InputStream protoInputStream) throws IOExcepti
               rule.addAllResourceFiles(a.getStringListValueList());
             } else if (a.getName().equals("manifest")) {
               rule.setManifest(a.getStringValue());
-            }
-
-            if (OTHER_ATTRIBUTES.contains(a.getName()) && !a.getStringValue().isEmpty()) {
-              rule.putOtherAttributes(a.getName(), a.getStringValue());
+            } else if (a.getName().equals("test_app")) {
+              rule.setTestApp(a.getStringValue());
+            } else if (a.getName().equals("instruments")) {
+              rule.setInstruments(a.getStringValue());
+            } else if (a.getName().equals("custom_package")) {
+              rule.setCustomPackage(a.getStringValue());
             }
           }
           ruleMap.put(target.getRule().getName(), rule.build());

diff --git a/querysync/java/com/google/idea/blaze/qsync/query/querysummary.proto b/querysync/java/com/google/idea/blaze/qsync/query/querysummary.proto
@@ -27,9 +27,11 @@ message Rule {
   repeated string sources = 2;
   repeated string deps = 3;
   repeated string idl_sources = 4;
-  // Other single-field attributes required by query sync
-  map<string, string> other_attributes = 5;
+  reserved 5;
   repeated string runtime_deps = 6;
   repeated string resource_files = 7;
   string manifest = 8;
+  string test_app = 9;
+  string instruments = 10;
+  string custom_package = 11;
 }