fix(datastore): PKG:datastore TYPE:datastoreClient FUNC:RunAggregationQuery (#7803)

MarioGravel · Mario Gravel · meredithslota · web-flow · commit 1f050ea92782 · 2023-06-21T14:34:28.000-04:00
Bug: #7800 Calling `func (c *datastore.Client) RunAggregationQuery` panics at line `789` when calling `c.client.RunAggregationQuery`. `c.client` is defined `type pb.DatastoreClient interface` and it should have `RunAggregationQuery(ctx context.Context, in *RunAggregationQueryRequest, opts ...grpc.CallOption) (*RunAggregationQueryResponse, error)` but, `c.client` is implemented as a `type datastoreClient struct` and is missing `func (dc *datastoreClient) RunAggregationQuery(ctx context.Context, in *pb.RunAggregationQueryRequest, opts ...grpc.CallOption) (res *pb.RunAggregationQueryResponse, err error)`. This bug was covered by the fact that `datastoreClient` complies to `type pb.DatastoreClient interface` because the interface is part of it struct declaration: ```go // From /datastore/client.go, line 36 ... type datastoreClient struct { // Embed so we still implement the DatastoreClient interface, // if the interface adds more methods. pb.DatastoreClient ... ``` Also(Bonus): `func (c *datastore.Client) RunAggregationQuery(ctx context.Context, aq *AggregationQuery) (AggregationResult, error)` returning an `type AggregationResult map[string]interface{}` but the content of type `interface{}` are `*datastore.Value` from `google.golang.org/genproto/googleapis/datastore/v1/entity.pb.go`. Not very useful. Solution: Add `func (dc *datastoreClient) RunAggregationQuery(ctx context.Context, in *pb.RunAggregationQueryRequest, opts ...grpc.CallOption) (res *pb.RunAggregationQueryResponse, err error)` Add a `switch` statement to use concrete values from the `GetXXXValue() YYY` of `google.golang.org/genproto/googleapis/datastore/v1 :: datastore.Value` Co-authored-by: Mario Gravel <info@MarioGravel.dev> Co-authored-by: meredithslota <meredithslota@google.com>
diff --git a/datastore/client.go b/datastore/client.go
@@ -73,6 +73,17 @@ func (dc *datastoreClient) RunQuery(ctx context.Context, in *pb.RunQueryRequest,
 	return res, err
 }
 
+func (dc *datastoreClient) RunAggregationQuery(ctx context.Context, in *pb.RunAggregationQueryRequest, opts ...grpc.CallOption) (res *pb.RunAggregationQueryResponse, err error) {
+	ctx = trace.StartSpan(ctx, "cloud.google.com/go/datastore.datastoreClient.RunAggregationQuery")
+	defer func() { trace.EndSpan(ctx, err) }()
+
+	err = dc.invoke(ctx, func(ctx context.Context) error {
+		res, err = dc.c.RunAggregationQuery(ctx, in, opts...)
+		return err
+	})
+	return res, nil
+}
+
 func (dc *datastoreClient) BeginTransaction(ctx context.Context, in *pb.BeginTransactionRequest, opts ...grpc.CallOption) (res *pb.BeginTransactionResponse, err error) {
 	ctx = trace.StartSpan(ctx, "cloud.google.com/go/datastore.datastoreClient.BeginTransaction")
 	defer func() { trace.EndSpan(ctx, err) }()
diff --git a/datastore/query.go b/datastore/query.go
@@ -796,7 +796,32 @@ func (c *Client) RunAggregationQuery(ctx context.Context, aq *AggregationQuery)
 	// TODO(developer): change batch parsing logic if other aggregations are supported.
 	for _, a := range res.Batch.AggregationResults {
 		for k, v := range a.AggregateProperties {
-			ar[k] = v
+			switch v.ValueType.(type) {
+			case *pb.Value_NullValue:
+				ar[k] = v.GetNullValue()
+			case *pb.Value_BooleanValue:
+				ar[k] = v.GetBooleanValue()
+			case *pb.Value_IntegerValue:
+				ar[k] = v.GetIntegerValue()
+			case *pb.Value_DoubleValue:
+				ar[k] = v.GetDoubleValue()
+			case *pb.Value_TimestampValue:
+				ar[k] = v.GetTimestampValue()
+			case *pb.Value_KeyValue:
+				ar[k] = v.GetKeyValue()
+			case *pb.Value_StringValue:
+				ar[k] = v.GetStringValue()
+			case *pb.Value_BlobValue:
+				ar[k] = v.GetBlobValue()
+			case *pb.Value_GeoPointValue:
+				ar[k] = v.GetGeoPointValue()
+			case *pb.Value_EntityValue:
+				ar[k] = v.GetEntityValue()
+			case *pb.Value_ArrayValue:
+				ar[k] = v.GetArrayValue()
+			default:
+				ar[k] = v
+			}
 		}
 	}
 
