Skip to content

Commit

Permalink
Fix cluster tests.
Browse files Browse the repository at this point in the history 
These tests were broken by clusters returning maps instead of arrays.
  • Loading branch information
@nihohit
nihohit committed Dec 12, 2023
1 parent b15da06 commit 8e98ea1
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 221 deletions.
43 changes: 25 additions & 18 deletions babushka-core/tests/test_cluster_client.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ mod utilities;

#[cfg(test)]
mod cluster_client_tests {
use std::collections::HashMap;

use super::*;
use babushka::connection_request::ReadFrom;
use redis::cluster_routing::{
Expand All @@ -11,17 +13,22 @@ mod cluster_client_tests {
use utilities::cluster::{setup_test_basics_internal, SHORT_CLUSTER_TEST_TIMEOUT};
use utilities::*;

fn count_primaries_and_replicas(info_replication: Vec<Vec<String>>) -> (u16, u16) {
fn count_primary_or_replica(value: &str) -> (u16, u16) {
if value.contains("role:master") {
(1, 0)
} else if value.contains("role:slave") {
(0, 1)
} else {
(0, 0)
}
}

fn count_primaries_and_replicas(info_replication: HashMap<String, String>) -> (u16, u16) {
info_replication
.into_iter()
.fold((0, 0), |acc, internal_vec| {
if internal_vec.iter().any(|str| str.contains("role:master")) {
(acc.0 + 1, acc.1)
} else if internal_vec.iter().any(|str| str.contains("role:slave")) {
(acc.0, acc.1 + 1)
} else {
(acc.0, acc.1)
}
.fold((0, 0), |acc, (_, value)| {
let count = count_primary_or_replica(&value);
(acc.0 + count.0, acc.1 + count.1)
})
}

Expand All @@ -43,7 +50,7 @@ mod cluster_client_tests {
.req_packed_command(&cmd, None)
.await
.unwrap();
let info = redis::from_redis_value::<Vec<Vec<String>>>(&info).unwrap();
let info = redis::from_redis_value::<HashMap<String, String>>(&info).unwrap();
let (primaries, replicas) = count_primaries_and_replicas(info);
assert_eq!(primaries, 3);
assert_eq!(replicas, 0);
Expand Down Expand Up @@ -74,7 +81,7 @@ mod cluster_client_tests {
)
.await
.unwrap();
let info = redis::from_redis_value::<Vec<Vec<String>>>(&info).unwrap();
let info = redis::from_redis_value::<HashMap<String, String>>(&info).unwrap();
let (primaries, replicas) = count_primaries_and_replicas(info);
assert_eq!(primaries, 3);
assert_eq!(replicas, 0);
Expand Down Expand Up @@ -105,7 +112,7 @@ mod cluster_client_tests {
)
.await
.unwrap();
let info = redis::from_redis_value::<Vec<Vec<String>>>(&info).unwrap();
let info = redis::from_redis_value::<HashMap<String, String>>(&info).unwrap();
let (primaries, replicas) = count_primaries_and_replicas(info);
assert_eq!(primaries, 3);
assert_eq!(replicas, 3);
Expand Down Expand Up @@ -135,8 +142,8 @@ mod cluster_client_tests {
)
.await
.unwrap();
let info = redis::from_redis_value::<Vec<Vec<String>>>(&info).unwrap();
let (primaries, replicas) = count_primaries_and_replicas(info);
let info = redis::from_redis_value::<String>(&info).unwrap();
let (primaries, replicas) = count_primary_or_replica(&info);
assert_eq!(primaries, 1);
assert_eq!(replicas, 0);
});
Expand Down Expand Up @@ -169,8 +176,8 @@ mod cluster_client_tests {
)
.await
.unwrap();
let info = redis::from_redis_value::<Vec<Vec<String>>>(&info).unwrap();
let (primaries, replicas) = count_primaries_and_replicas(info);
let info = redis::from_redis_value::<String>(&info).unwrap();
let (primaries, replicas) = count_primary_or_replica(&info);
assert_eq!(primaries, 0);
assert_eq!(replicas, 1);
});
Expand Down Expand Up @@ -203,8 +210,8 @@ mod cluster_client_tests {
)
.await
.unwrap();
let info = redis::from_redis_value::<Vec<Vec<String>>>(&info).unwrap();
let (primaries, replicas) = count_primaries_and_replicas(info);
let info = redis::from_redis_value::<String>(&info).unwrap();
let (primaries, replicas) = count_primary_or_replica(&info);
assert_eq!(primaries, 0);
assert_eq!(replicas, 1);
});
Expand Down
8 changes: 2 additions & 6 deletions babushka-core/tests/test_socket_listener.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -617,16 +617,12 @@ mod socket_listener {
};
let pointer = pointer as *mut Value;
let received_value = unsafe { Box::from_raw(pointer) };
let Value::Array(values) = *received_value else {
let Value::Map(values) = *received_value else {
panic!("Unexpected value {:?}", received_value);
};
assert_eq!(values.len(), 3);
for i in 0..3 {
let Value::Array(nested_values) = values.get(i).unwrap() else {
panic!("Unexpected value {:?}", values[i]);
};
assert_eq!(nested_values.len(), 2);
assert_eq!(nested_values[1], Value::BulkString(b"foo".to_vec()));
assert_eq!(values.get(i).unwrap().1, Value::BulkString(b"foo".to_vec()));
}
}

Expand Down
53 changes: 4 additions & 49 deletions node/src/RedisClusterClient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,25 +122,6 @@ function toProtobufRoute(
}
}

/** Convert the multi-node response from a list of [address, nodeResponse] pairs to
* a dictionary where each address is the key and its corresponding node response is the value.
*
* @param response - A list of lists, where each inner list contains an address (string)
* and the corresponding node response (of type T). Or a single node response (of type T).
* @param isSingleResponse - Predicate that checks if `response` is single node response.
* @returns `response` if response is single node response,
* otherwise a dictionary where each address is the key and its corresponding node response is the value.
*/
export function convertMultiNodeResponseToDict<T>(
response: T | [string, T][],
isSingleResponse: (res: T | [string, T][]) => boolean
): T | Record<string, T> {
if (isSingleResponse(response)) {
return response as T;
}
return Object.fromEntries(response as [string, T][]);
}

export class RedisClusterClient extends BaseClient {
/**
* @internal
Expand Down Expand Up @@ -241,16 +222,10 @@ export class RedisClusterClient extends BaseClient {
options?: InfoOptions[],
route?: Routes
): Promise<ClusterResponse<string>> {
const result = this.createWritePromise<string | [string, string][]>(
return this.createWritePromise<ClusterResponse<string>>(
createInfo(options),
toProtobufRoute(route)
);
return result.then((res) => {
return convertMultiNodeResponseToDict<string>(
res,
(response) => typeof response == "string"
);
});
}

/** Get the name of the current connection.
Expand All @@ -266,16 +241,10 @@ export class RedisClusterClient extends BaseClient {
public clientGetName(
route?: Routes
): Promise<ClusterResponse<string | null>> {
const result = this.createWritePromise<string | null>(
return this.createWritePromise<ClusterResponse<string | null>>(
createClientGetName(),
toProtobufRoute(route)
);
return result.then((res) => {
return convertMultiNodeResponseToDict<string | null>(
res,
(response) => typeof response == "string" || response == null
);
});
}

/** Rewrite the configuration file with the current configuration.
Expand Down Expand Up @@ -317,16 +286,10 @@ export class RedisClusterClient extends BaseClient {
* it returns a dictionary where each address is the key and its corresponding node response is the value.
*/
public clientId(route?: Routes): Promise<ClusterResponse<number>> {
const result = this.createWritePromise<number>(
return this.createWritePromise<ClusterResponse<number>>(
createClientId(),
toProtobufRoute(route)
);
return result.then((res) => {
return convertMultiNodeResponseToDict<number>(
res,
(response) => typeof response == "number"
);
});
}

/** Reads the configuration parameters of a running Redis server.
Expand All @@ -344,18 +307,10 @@ export class RedisClusterClient extends BaseClient {
parameters: string[],
route?: Routes
): Promise<ClusterResponse<string[]>> {
const result = this.createWritePromise<string[] | [string, string[]][]>(
return this.createWritePromise<ClusterResponse<string[]>>(
createConfigGet(parameters),
toProtobufRoute(route)
);
return result.then((res) => {
return convertMultiNodeResponseToDict<string[]>(
res,
(response: (string | [string, string[]])[]) =>
Array.isArray(response) &&
response.every((item) => typeof item === "string")
);
});
}

/** Set configuration parameters to the specified values.
Expand Down
48 changes: 1 addition & 47 deletions node/tests/RedisClusterClient.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ import {
BaseClientConfiguration,
ClusterTransaction,
InfoOptions,
RedisClusterClient
RedisClusterClient,
} from "../";
import { convertMultiNodeResponseToDict } from "../src/RedisClusterClient";
import { runBaseTests } from "./SharedTests";
import { flushallOnPort, transactionTest } from "./TestUtilities";

Expand Down Expand Up @@ -222,49 +221,4 @@ describe("RedisClusterClient", () => {
},
TIMEOUT
);

it(
"convertMultiNodeResponseToDict function test",
async () => {
const param1 = "This is a string value";
const param2 = ["value", "value"];
const param3 = [
["value1", ["value"]],
["value2", ["value"]],
] as [string, string[]][];
const result = { value1: ["value"], value2: ["value"] };

const isString = (response: string | [string, string][]) =>
typeof response == "string";

const isNull = (response: null | [string, null][]) =>
response == null;

const isStringArray = (
response: (string | [string, string[]])[]
): boolean => {
return (
Array.isArray(response) &&
response.every((item) => typeof item === "string")
);
};

expect(
convertMultiNodeResponseToDict<string>(param1, isString)
).toEqual(param1);

expect(
convertMultiNodeResponseToDict<string[]>(param2, isStringArray)
).toEqual(param2);

expect(
convertMultiNodeResponseToDict<string[]>(param3, isStringArray)
).toEqual(result);

expect(
convertMultiNodeResponseToDict<null>(null, isNull)
).toBeNull();
},
TIMEOUT
);
});
Loading

0 comments on commit 8e98ea1

Please sign in to comment.