Skip to content

indexer: fix bound checks in getDrizzleCursor #109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fracek merged 1 commit into from
Oct 22, 2024
Merged

indexer: fix bound checks in getDrizzleCursor #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions change/@apibara-indexer-2ad98856-798e-4946-8109-02388d49f0e4.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "indexer: fix conditions in getDrizzleCursor",
"packageName": "@apibara/indexer",
"email": "jadejajaipal5@gmail.com",
"dependentChangeType": "patch"
}
9 changes: 3 additions & 6 deletions packages/indexer/src/sinks/drizzle/drizzle.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,13 @@ import { drizzle } from "drizzle-orm/node-postgres";
import { serial, text } from "drizzle-orm/pg-core";
import { Client } from "pg";
import { beforeAll, beforeEach, describe, expect, it } from "vitest";
import {
type Int8Range,
drizzle as drizzleSink,
getDrizzleCursor,
pgTable,
} from ".";
import { useSink } from "../../hooks";
import { run } from "../../indexer";
import { generateMockMessages } from "../../testing";
import { getMockIndexer } from "../../testing/indexer";
import type { Int8Range } from "./Int8Range";
import { drizzle as drizzleSink } from "./drizzle";
import { getDrizzleCursor, pgTable } from "./utils";
Comment on lines +16 to +18
Copy link

@coderabbitai coderabbitai bot Oct 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Incomplete Refactoring: Old Import References Detected

The refactoring appears to be incomplete as there are still references to the old import locations in several files:

  • packages/indexer/src/sinks/drizzle/utils.ts
  • packages/indexer/src/sinks/drizzle/drizzle.test.ts
  • packages/indexer/src/sinks/drizzle/index.ts
  • packages/indexer/src/sinks/drizzle/insert.ts

Please update these references to use the new import paths and ensure that all tests pass with the updated structure.

🔗 Analysis chain

Verify the refactoring impact on other parts of the codebase.

The new imports suggest a refactoring of the project structure, moving some functionality to separate files. While the changes in this file look good and all tests seem to be using the new imports correctly, it's important to ensure that this refactoring hasn't negatively impacted other parts of the codebase.

To verify the impact of these changes, you can run the following script:

This script will help identify any missed import updates, confirm the existence of the new utility files, and verify that all tests still pass with the new changes.

[/run_scripts]

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining references to the old import locations

# Search for any remaining references to the old import locations
echo "Searching for old import references:"
rg -n 'from "\./Int8Range"' --type ts
rg -n 'from "\./drizzle"' --type ts
rg -n 'getDrizzleCursor|pgTable.*from "\./' --type ts

# Check if the new utility files exist
echo "Checking for new utility files:"
ls -l ./Int8Range.ts ./drizzle.ts ./utils.ts

# Run the test suite to ensure all tests pass with the new changes
echo "Running test suite:"
npm test

Length of output: 2124

Script:

#!/bin/bash
# Description: Re-verify the existence of new utility files in the correct directory

# Define the base directory based on the initial file location
BASE_DIR="packages/indexer/src/sinks/drizzle"

# Check if the new utility files exist in the expected directory
echo "Checking for new utility files in ${BASE_DIR}:"
ls -l ${BASE_DIR}/Int8Range.ts ${BASE_DIR}/drizzle.ts ${BASE_DIR}/utils.ts

# Search for any remaining references to the old import locations within the correct directory
echo "Searching for old import references in ${BASE_DIR}:"
rg -n 'from "\./Int8Range"' --type ts ${BASE_DIR}
rg -n 'from "\./drizzle"' --type ts ${BASE_DIR}
rg -n 'getDrizzleCursor|pgTable.*from "\./' --type ts ${BASE_DIR}

Length of output: 2499

Copy link
Contributor

@fracek fracek Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new code is better

Copy link

@coderabbitai coderabbitai bot Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fracek, understood. Let me know if I can assist further.

🐰😢

✏️ Learnings added 
Learnt from: fracek
PR: apibara/typescript-sdk#109
File: packages/indexer/src/sinks/drizzle/drizzle.test.ts:16-18
Timestamp: 2024-10-22T11:16:00.552Z
Learning: In `packages/indexer/src/sinks/drizzle`, the import references are correct and up-to-date according to the new code structure.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


const testTable = pgTable("test_table", {
id: serial("id").primaryKey(),
Expand Down
4 changes: 2 additions & 2 deletions packages/indexer/src/sinks/drizzle/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ export const getDrizzleCursor = (
const isArray = Array.isArray(cursor_range);
const [lower, upper] = isArray ? cursor_range : [cursor_range, undefined];
let isNoUpperBound = false;
if (!lower) {
if (lower === undefined) {
Copy link
Contributor

@fracek fracek Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wish there was a lint for this

throw new Error("Lower bound cursor is required");
}
if (!upper) {
if (upper === undefined) {
isNoUpperBound = true;
}
return new Int8Range(
Expand Down
Loading