Skip to content

Commit

Permalink
Node: Add style lint requiring empty lines. (valkey-io#753)
Browse files Browse the repository at this point in the history
The empty lines will be added around functions, classes, and multi-line
blocks, such as large `if` statements.

Co-authored-by: nihohit <[email protected]>
  • Loading branch information
shachlanAmazon and nihohit authored Jan 2, 2024
1 parent bedb06e commit 85d7056
Show file tree
Hide file tree
Showing 11 changed files with 146 additions and 56 deletions.
9 changes: 9 additions & 0 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,14 @@ module.exports = {
rules: {
"tsdoc/syntax": "error",
"import/no-unresolved": "off",
"padding-line-between-statements": [
"error",
{ blankLine: "always", prev: "*", next: "class" },
{ blankLine: "always", prev: "class", next: "*" },
{ blankLine: "always", prev: "*", next: "function" },
{ blankLine: "always", prev: "function", next: "*" },
{ blankLine: "always", prev: "*", next: "multiline-block-like" },
{ blankLine: "always", prev: "multiline-block-like", next: "*" },
],
},
};
7 changes: 7 additions & 0 deletions benchmarks/node/node_benchmark.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@ function choose_action(): ChosenAction {
if (Math.random() > PROB_GET) {
return ChosenAction.SET;
}

if (Math.random() > PROB_GET_EXISTING_KEY) {
return ChosenAction.GET_NON_EXISTING;
}

return ChosenAction.GET_EXISTING;
}

Expand All @@ -66,6 +68,7 @@ async function redis_benchmark(
const chosen_action = choose_action();
const tic = process.hrtime();
const client = clients[started_tasks_counter % clients.length];

switch (chosen_action) {
case ChosenAction.GET_EXISTING:
await client.get(generate_key_set());
Expand All @@ -77,6 +80,7 @@ async function redis_benchmark(
await client.set(generate_key_set(), data);
break;
}

const toc = process.hrtime(tic);
const latency_list = action_latencies[chosen_action];
latency_list.push(toc[0] * 1000 + toc[1] / 1000000);
Expand All @@ -92,11 +96,13 @@ async function create_bench_tasks(
) {
started_tasks_counter = 0;
const tic = process.hrtime();

for (let i = 0; i < num_of_concurrent_tasks; i++) {
running_tasks.push(
redis_benchmark(clients, total_commands, data, action_latencies)
);
}

await Promise.all(running_tasks);
const toc = process.hrtime(tic);
return toc[0] + toc[1] / 1000000000;
Expand Down Expand Up @@ -202,6 +208,7 @@ async function main(
port: number
) {
const data = generate_value(data_size);

if (clients_to_run == "all" || clients_to_run == "glide") {
const clientClass = clusterModeEnabled
? RedisClusterClient
Expand Down
17 changes: 16 additions & 1 deletion node/src/BaseClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,15 @@ function getRequestErrorClass(
if (type === response.RequestErrorType.Disconnect) {
return ConnectionError;
}

if (type === response.RequestErrorType.ExecAbort) {
return ExecAbortError;
}

if (type === response.RequestErrorType.Timeout) {
return TimeoutError;
}

if (type === response.RequestErrorType.Unspecified) {
return RequestError;
}
Expand All @@ -190,9 +193,11 @@ export class BaseClient {
: data;
let lastPos = 0;
const reader = Reader.create(buf);

while (reader.pos < reader.len) {
lastPos = reader.pos;
let message = undefined;

try {
message = response.Response.decodeDelimited(reader);
} catch (err) {
Expand All @@ -208,13 +213,16 @@ export class BaseClient {
return;
}
}

if (message.closingError != null) {
this.close(message.closingError);
return;
}

const [resolve, reject] =
this.promiseCallbackFunctions[message.callbackIdx];
this.availableCallbackSlots.push(message.callbackIdx);

if (message.requestError != null) {
const errorType = getRequestErrorClass(
message.requestError.type
Expand All @@ -224,6 +232,7 @@ export class BaseClient {
);
} else if (message.respPointer != null) {
const pointer = message.respPointer;

if (typeof pointer === "number") {
resolve(valueFromSplitPointer(0, pointer));
} else {
Expand All @@ -237,6 +246,7 @@ export class BaseClient {
resolve(null);
}
}

this.remainingReadData = undefined;
}

Expand Down Expand Up @@ -289,8 +299,11 @@ export class BaseClient {
route?: redis_request.Routes
): Promise<T> {
if (this.isClosed) {
throw new ClosingError("Unable to execute requests; the client is closed. Please create a new client.");
throw new ClosingError(
"Unable to execute requests; the client is closed. Please create a new client."
);
}

return new Promise((resolve, reject) => {
const callbackIndex = this.getCallbackIndex();
this.promiseCallbackFunctions[callbackIndex] = [resolve, reject];
Expand Down Expand Up @@ -329,9 +342,11 @@ export class BaseClient {
encodeDelimited: (message: TRequest, writer: Writer) => void
) {
encodeDelimited(message, this.requestWriter);

if (this.writeInProgress) {
return;
}

this.writeBufferedRequestsToSocket();
}

Expand Down
11 changes: 11 additions & 0 deletions node/src/Commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,30 @@ import RequestType = redis_request.RequestType;

function isLargeCommand(args: string[]) {
let lenSum = 0;

for (const arg of args) {
lenSum += arg.length;

if (lenSum >= MAX_REQUEST_ARGS_LEN) {
return true;
}
}

return false;
}

export function parseInfoResponse(response: string): Record<string, string> {
const lines = response.split("\n");
const parsedResponse: Record<string, string> = {};

for (const line of lines) {
// Ignore lines that start with '#'
if (!line.startsWith("#")) {
const [key, value] = line.trim().split(":");
parsedResponse[key] = value;
}
}

return parsedResponse;
}

Expand All @@ -48,6 +53,7 @@ function createCommand(
args: args,
});
}

return singleCommand;
}

Expand Down Expand Up @@ -108,15 +114,18 @@ export function createSet(
options?: SetOptions
): redis_request.Command {
const args = [key, value];

if (options) {
if (options.conditionalSet === "onlyIfExists") {
args.push("XX");
} else if (options.conditionalSet === "onlyIfDoesNotExist") {
args.push("NX");
}

if (options.returnOldValue) {
args.push("GET");
}

if (
options.expiry &&
options.expiry !== "keepExisting" &&
Expand All @@ -128,6 +137,7 @@ export function createSet(
)}'. Count must be an integer`
);
}

if (options.expiry === "keepExisting") {
args.push("KEEPTTL");
} else if (options.expiry?.type === "seconds") {
Expand All @@ -140,6 +150,7 @@ export function createSet(
args.push("PXAT " + options.expiry.count);
}
}

return createCommand(RequestType.SetString, args);
}

Expand Down
1 change: 1 addition & 0 deletions node/src/Logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export class Logger {
if (!Logger._instance) {
new Logger();
}

const level = LEVEL.get(logLevel) || 0;
if (!(level <= Logger.logger_level)) return;
log(level, logIdentifier, message);
Expand Down
1 change: 1 addition & 0 deletions node/src/RedisClusterClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ function toProtobufRoute(
if (route === undefined) {
return undefined;
}

if (route === "allPrimaries") {
return redis_request.Routes.create({
simpleRoutes: redis_request.SimpleRoutes.AllPrimaries,
Expand Down
9 changes: 9 additions & 0 deletions node/tests/RedisClientInternals.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ function createLeakedValue(value: ReturnType): Long {
if (value == null) {
return new Long(0, 0);
}

let pair = [0, 0];

if (typeof value === "string") {
pair = createLeakedString(value);
} else if (value instanceof Array) {
Expand Down Expand Up @@ -94,6 +96,7 @@ function sendResponse(
) {
const new_response = response.Response.create();
new_response.callbackIdx = callbackIndex;

if (responseType == ResponseType.Value) {
const pointer = createLeakedValue(response_data?.value ?? "fake data");
new_response.respPointer = pointer;
Expand All @@ -111,6 +114,7 @@ function sendResponse(
} else {
throw new Error("Got unknown response type: " + responseType);
}

const response_bytes =
response.Response.encodeDelimited(new_response).finish();
socket.write(response_bytes);
Expand Down Expand Up @@ -139,11 +143,13 @@ function getConnectionAndSocket(
connection_request.ConnectionRequest.decodeDelimited(
reader
);

if (checkRequest && !checkRequest(request)) {
reject(
`${JSON.stringify(request)} did not pass condition`
);
}

sendResponse(socket, ResponseType.Null, 0);
});

Expand Down Expand Up @@ -482,9 +488,11 @@ describe("SocketConnectionInternals", () => {
RequestType.SetString
);
const args = request.singleCommand?.argsArray?.args;

if (!args) {
throw new Error("no args");
}

expect(args.length).toEqual(5);
expect(args[0]).toEqual("foo");
expect(args[1]).toEqual("bar");
Expand Down Expand Up @@ -586,6 +594,7 @@ describe("SocketConnectionInternals", () => {
expect(request.singleCommand?.requestType).toEqual(
RequestType.CustomCommand
);

if (request.singleCommand?.argsArray?.args?.at(0) === "SET") {
expect(request.route?.simpleRoutes).toEqual(
redis_request.SimpleRoutes.AllPrimaries
Expand Down
20 changes: 13 additions & 7 deletions node/tests/RedisClusterClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,22 @@ import {
beforeAll,
describe,
expect,
it
it,
} from "@jest/globals";
import {
BaseClientConfiguration,
ClusterTransaction,
InfoOptions,
ProtocolVersion,
RedisClusterClient
RedisClusterClient,
} from "../";
import { runBaseTests } from "./SharedTests";
import { RedisCluster, flushallOnPort, getFirstResult, transactionTest } from "./TestUtilities";
import {
RedisCluster,
flushallOnPort,
getFirstResult,
transactionTest,
} from "./TestUtilities";

type Context = {
client: RedisClusterClient;
Expand Down Expand Up @@ -66,6 +71,7 @@ describe("RedisClusterClient", () => {
if (testSucceeded) {
testsFailed -= 1;
}

context.client.close();
},
timeout: TIMEOUT,
Expand All @@ -77,9 +83,9 @@ describe("RedisClusterClient", () => {
const client = await RedisClusterClient.createClient(
getOptions(cluster.ports())
);
const info_server = getFirstResult(await client.info([
InfoOptions.Server,
]));
const info_server = getFirstResult(
await client.info([InfoOptions.Server])
);
expect(info_server).toEqual(expect.stringContaining("# Server"));

const result = (await client.info([
Expand Down Expand Up @@ -145,7 +151,7 @@ describe("RedisClusterClient", () => {
);
const transaction = new ClusterTransaction();
const expectedRes = transactionTest(transaction);
const result = await client.exec(transaction)
const result = await client.exec(transaction);
expect(result).toEqual(expectedRes);
client.close();
},
Expand Down
Loading

0 comments on commit 85d7056

Please sign in to comment.