Skip to content

Commit

Permalink
Fix Configuration GRPC calls not sending metadata (#571)
Browse files Browse the repository at this point in the history
* Fix Configuration GRPC calls not sending metadata

Signed-off-by: Trevor Lund <[email protected]>

* Address PR comment

Signed-off-by: Trevor Lund <[email protected]>

---------

Signed-off-by: Trevor Lund <[email protected]>
  • Loading branch information
tlund101 authored Feb 12, 2024
1 parent 5c2b40a commit 319e2fb
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 25 deletions.
25 changes: 5 additions & 20 deletions src/implementation/Client/GRPCClient/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ limitations under the License.
*/

import GRPCClient from "./GRPCClient";
import * as grpc from "@grpc/grpc-js";
import {
GetConfigurationRequest,
GetConfigurationResponse,
Expand All @@ -28,7 +27,7 @@ import { SubscribeConfigurationResponse as SubscribeConfigurationResponseResult
import { SubscribeConfigurationCallback } from "../../../types/configuration/SubscribeConfigurationCallback";
import { SubscribeConfigurationStream } from "../../../types/configuration/SubscribeConfigurationStream";
import { ConfigurationItem } from "../../../types/configuration/ConfigurationItem";
import { createConfigurationType } from "../../../utils/Client.util";
import { addMetadataToMap, createConfigurationType } from "../../../utils/Client.util";

export default class GRPCClientConfiguration implements IClientConfiguration {
client: GRPCClient;
Expand All @@ -38,25 +37,18 @@ export default class GRPCClientConfiguration implements IClientConfiguration {
}

async get(storeName: string, keys: string[], metadataObj?: KeyValueType): Promise<GetConfigurationResponseResult> {
const metadata = new grpc.Metadata();

const msg = new GetConfigurationRequest();
msg.setStoreName(storeName);

if (keys && keys.length > 0) {
msg.setKeysList(keys.filter((i) => i !== ""));
}

if (metadataObj) {
for (const [key, value] of Object.entries(metadataObj)) {
metadata.add(key, value);
}
}
addMetadataToMap(msg.getMetadataMap(), metadataObj);

const client = await this.client.getClient();

return new Promise((resolve, reject) => {
client.getConfiguration(msg, metadata, (err, res: GetConfigurationResponse) => {
client.getConfiguration(msg, (err, res: GetConfigurationResponse) => {
if (err) {
return reject(err);
}
Expand Down Expand Up @@ -99,8 +91,6 @@ export default class GRPCClientConfiguration implements IClientConfiguration {
keys?: string[],
metadataObj?: KeyValueType,
): Promise<SubscribeConfigurationStream> {
const metadata = new grpc.Metadata();

const msg = new SubscribeConfigurationRequest();
msg.setStoreName(storeName);

Expand All @@ -109,20 +99,15 @@ export default class GRPCClientConfiguration implements IClientConfiguration {
} else {
msg.setKeysList([]);
}

if (metadataObj) {
for (const [key, value] of Object.entries(metadataObj)) {
metadata.add(key, value);
}
}
addMetadataToMap(msg.getMetadataMap(), metadataObj);

const client = await this.client.getClient();

// Open a stream. Note that this is a never-ending stream
// and will stay open as long as the client is open
// we will thus create a set with our listeners so we don't
// break on multi listeners
const stream = client.subscribeConfiguration(msg, metadata);
const stream = client.subscribeConfiguration(msg);
let streamId: string;

stream.on("data", async (data: SubscribeConfigurationResponse) => {
Expand Down
7 changes: 2 additions & 5 deletions test/e2e/grpc/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,14 +363,11 @@ describe("grpc/client", () => {
});

it("should be able to get the configuration items with metadata", async () => {
await client.configuration.get("config-redis", ["myconfigkey1"], {
const conf = await client.configuration.get("config-redis", ["myconfigkey1"], {
hello: "world",
});

// Disabled for now as I am unsure if Dapr returns the metadata items
// Java SDK: https://github.com/dapr/java-sdk/blob/06d92dafca62a6b48e74ccf939feeac7189e360f/sdk/src/test/java/io/dapr/client/DaprPreviewClientGrpcTest.java#L119
// ^ shows that it is not being tested, it tries but doesn't assert
// expect(conf.items.filter(i => i.key == "myconfigkey1")[0].metadata).toHaveProperty("hello");
expect(conf.items["myconfigkey1"].metadata).toHaveProperty("hello");
});

it("should be able to subscribe to configuration item changes on all keys", async () => {
Expand Down

0 comments on commit 319e2fb

Please sign in to comment.