Skip to content

feat: add missing data checker (works in conjunction with archiver mi…#87

Open
PudgyPug wants to merge 2 commits intodevfrom
missing-data-checker
Open

feat: add missing data checker (works in conjunction with archiver mi…#87
PudgyPug wants to merge 2 commits intodevfrom
missing-data-checker

Conversation

@PudgyPug
Copy link

@PudgyPug PudgyPug commented May 13, 2025

User description

…ssing data script)


PR Type

Enhancement


Description

  • Introduces a script to check for missing data in the database

  • Reads a JSON file with expected data and verifies existence in storage

  • Outputs a JSON file listing still-missing records

  • Supports cycles, receipts, accounts, and transactions tables


Changes walkthrough 📝

Relevant files
Enhancement
checkMissingData.ts
Add script to check and report missing database records   

scripts/checkMissingData.ts

  • Adds a new script to verify missing data in database tables
  • Reads input JSON of expected records and checks their presence
  • Outputs a JSON file with records still missing after check
  • Handles cycles, receipts, accounts, and transactions
  • +100/-0 

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🏅 Score: 78
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Missing Await

    The calls to Receipt.queryReceiptByReceiptId and Account.queryAccountByAccountId are not awaited, which may result in unexpected behavior if these functions are asynchronous. This should be checked and corrected if necessary.

      const receiptRecord = Receipt.queryReceiptByReceiptId(receipt.id)
      if (!receiptRecord) {
        console.log(`Missing receipt record for receipt ${receipt.id}`)
        nonPatchedData.receipts.push(receipt)
      } else {
        console.log(`Receipt record for receipt ${receipt.id} exists`)
      }
    }
    // check transactions table
    for (const transaction of missingData.transactions) {
      const transactionRecord = await Transaction.queryTransactionByTxId(transaction.id)
      if (!transactionRecord) {
        console.log(`Missing transaction record for transaction ${transaction.id}`)
        nonPatchedData.transactions.push(transaction)
      } else {
        console.log(`Transaction record for transaction ${transaction.id} exists`)
      }
    }
    // check accounts table
    for (const account of missingData.accounts) {
      const accountRecord = Account.queryAccountByAccountId(account.id)
      if (!accountRecord) {
    Unmocked Side Effects

    The script directly interacts with the database and filesystem. If tests are added, ensure all external side effects (DB, FS) are properly mocked to avoid unintended consequences during testing.

    async function main(): Promise<void> {
      overrideDefaultConfig(process.env, process.argv)
      Crypto.setCryptoHashKey(config.hashKey)
      await Storage.initializeDB()
    
      // read missingDataFile that's a json file
      const missingDataFile = scriptConfig.files.inputFile
      // eslint-disable-next-line security/detect-non-literal-fs-filename
      const missingData = JSON.parse(readFileSync(missingDataFile, 'utf8')) as MissingData
      if (!missingData) {
        console.error('No missing data found in the file.')
        return
      }
    
      const nonPatchedData: MissingData = {
        cycles: [],
        receipts: [],
        accounts: [],
        transactions: [],
        timestamp: Date.now(),
      }
      // check cycles table
      for (const cycle of missingData.cycles) {
        const cycleRecord = await Cycle.queryCycleByCounter(cycle.counter)
        if (!cycleRecord) {
          console.log(`Missing cycle record for cycle ${cycle.counter}`)
          nonPatchedData.cycles.push(cycle)
        } else {
          console.log(`Cycle record for cycle ${cycle.counter} exists`)
        }
      }
    
      // check receipts table
      for (const receipt of missingData.receipts) {
        const receiptRecord = Receipt.queryReceiptByReceiptId(receipt.id)
        if (!receiptRecord) {
          console.log(`Missing receipt record for receipt ${receipt.id}`)
          nonPatchedData.receipts.push(receipt)
        } else {
          console.log(`Receipt record for receipt ${receipt.id} exists`)
        }
      }
      // check transactions table
      for (const transaction of missingData.transactions) {
        const transactionRecord = await Transaction.queryTransactionByTxId(transaction.id)
        if (!transactionRecord) {
          console.log(`Missing transaction record for transaction ${transaction.id}`)
          nonPatchedData.transactions.push(transaction)
        } else {
          console.log(`Transaction record for transaction ${transaction.id} exists`)
        }
      }
      // check accounts table
      for (const account of missingData.accounts) {
        const accountRecord = Account.queryAccountByAccountId(account.id)
        if (!accountRecord) {
          console.log(`Missing account record for account ${account.id}`)
          nonPatchedData.accounts.push(account)
        } else {
          console.log(`Account record for account ${account.id} exists`)
        }
      }
    
      // write nonPatchedData to missingDataFile
      const outputFile = scriptConfig.files.outputFile
      // eslint-disable-next-line security/detect-non-literal-fs-filename
      writeFileSync(outputFile, JSON.stringify(nonPatchedData, null, 2), 'utf8')
      console.log(`Missing data written to ${outputFile}`)
      // close database
      Storage.closeDatabase()
      console.log('Database closed')
      console.log('Script completed successfully')
      process.exit(0)
    }

    Comment on lines +59 to +65
    const receiptRecord = Receipt.queryReceiptByReceiptId(receipt.id)
    if (!receiptRecord) {
    console.log(`Missing receipt record for receipt ${receipt.id}`)
    nonPatchedData.receipts.push(receipt)
    } else {
    console.log(`Receipt record for receipt ${receipt.id} exists`)
    }

    Choose a reason for hiding this comment

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

    Suggestion: The call to Receipt.queryReceiptByReceiptId is likely asynchronous, but it is not awaited. This can cause receiptRecord to be a Promise, leading to incorrect logic and missed missing receipts. [possible issue, importance: 8]

    Suggested change
    const receiptRecord = Receipt.queryReceiptByReceiptId(receipt.id)
    if (!receiptRecord) {
    console.log(`Missing receipt record for receipt ${receipt.id}`)
    nonPatchedData.receipts.push(receipt)
    } else {
    console.log(`Receipt record for receipt ${receipt.id} exists`)
    }
    const receiptRecord = await Receipt.queryReceiptByReceiptId(receipt.id)
    if (!receiptRecord) {
    console.log(`Missing receipt record for receipt ${receipt.id}`)
    nonPatchedData.receipts.push(receipt)
    } else {
    console.log(`Receipt record for receipt ${receipt.id} exists`)
    }

    Comment on lines +79 to +85
    const accountRecord = Account.queryAccountByAccountId(account.id)
    if (!accountRecord) {
    console.log(`Missing account record for account ${account.id}`)
    nonPatchedData.accounts.push(account)
    } else {
    console.log(`Account record for account ${account.id} exists`)
    }

    Choose a reason for hiding this comment

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

    Suggestion: The call to Account.queryAccountByAccountId is likely asynchronous, but it is not awaited. This may cause logic errors and incorrect detection of missing accounts. [possible issue, importance: 8]

    Suggested change
    const accountRecord = Account.queryAccountByAccountId(account.id)
    if (!accountRecord) {
    console.log(`Missing account record for account ${account.id}`)
    nonPatchedData.accounts.push(account)
    } else {
    console.log(`Account record for account ${account.id} exists`)
    }
    const accountRecord = await Account.queryAccountByAccountId(account.id)
    if (!accountRecord) {
    console.log(`Missing account record for account ${account.id}`)
    nonPatchedData.accounts.push(account)
    } else {
    console.log(`Account record for account ${account.id} exists`)
    }

    Comment on lines +94 to +97
    Storage.closeDatabase()
    console.log('Database closed')
    console.log('Script completed successfully')
    process.exit(0)

    Choose a reason for hiding this comment

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

    Suggestion: Storage.closeDatabase() is likely asynchronous, but it is not awaited. Not awaiting it may cause the process to exit before the database is properly closed, risking data corruption or resource leaks. [possible issue, importance: 7]

    Suggested change
    Storage.closeDatabase()
    console.log('Database closed')
    console.log('Script completed successfully')
    process.exit(0)
    await Storage.closeDatabase()
    console.log('Database closed')
    console.log('Script completed successfully')
    process.exit(0)

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    1 participant