From 509ea5ec4c5a164eae3cd5e72900684e3994d066 Mon Sep 17 00:00:00 2001 From: Jay Ji Date: Mon, 29 Jul 2024 09:45:12 +1200 Subject: [PATCH] Fix get bigInt from jsonb, and deepclone from cache model (#2504) * Fix get bigInt from jsonb, and deepclone from cache model * Update packages/node-core/src/utils/graphql.ts Co-authored-by: Scott Twiname * Update packages/node-core/src/utils/graphql.ts Co-authored-by: Scott Twiname * update --------- Co-authored-by: Scott Twiname --- packages/node-core/CHANGELOG.md | 4 ++ .../src/indexer/storeCache/cacheModel.test.ts | 66 ++++++++++++------- .../src/indexer/storeCache/cacheModel.ts | 36 +++++----- packages/node-core/src/utils/graphql.ts | 36 ++++++++-- 4 files changed, 93 insertions(+), 49 deletions(-) diff --git a/packages/node-core/CHANGELOG.md b/packages/node-core/CHANGELOG.md index 6272b50d05..a61f5bfd81 100644 --- a/packages/node-core/CHANGELOG.md +++ b/packages/node-core/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- Fixed get and set data not been deepCloned and data is not mutable +- Improved get bigInt type from json type + ## [13.0.0] - 2024-07-25 ### Changed - Breaking change: Require indexing environment timezone set to UTC, avoid inconsistent result from cache and database (#2495) diff --git a/packages/node-core/src/indexer/storeCache/cacheModel.test.ts b/packages/node-core/src/indexer/storeCache/cacheModel.test.ts index b65eb8ad16..7398d45839 100644 --- a/packages/node-core/src/indexer/storeCache/cacheModel.test.ts +++ b/packages/node-core/src/indexer/storeCache/cacheModel.test.ts @@ -3,7 +3,7 @@ import {GraphQLModelsType} from '@subql/utils'; import {Sequelize, DataTypes, QueryTypes} from '@subql/x-sequelize'; -import {padStart} from 'lodash'; +import {cloneDeep, padStart} from 'lodash'; import {DbOption, modelsTypeToModelAttributes, NodeConfig} from '../../'; import {CachedModel} from './cacheModel'; @@ -378,20 +378,24 @@ describe('cacheModel integration', () => { await sequelize.close(); }); + async function setDefaultData(id: string, height: number, data?: any): Promise { + cacheModel.set( + id, + data ?? { + id, + selfStake: BigInt(1000000000000000000000n), + oneEntity: {testItem: 'test', amount: BigInt(8000000000000000000000n)}, + delegators: [{delegator: '0x02', amount: BigInt(1000000000000000000000n)}], + randomNArray: [1, 2, 3, 4, 5], + }, + height + ); + await flush(height + 1); + } + describe('cached data and db data compare', () => { it('bigint value in jsonb', async () => { - cacheModel.set( - `0x01`, - { - id: `0x01`, - selfStake: BigInt(1000000000000000000000n), - oneEntity: {testItem: 'test', amount: BigInt(8000000000000000000000n)}, - delegators: [{delegator: '0x02', amount: BigInt(1000000000000000000000n)}], - randomNArray: [1, 2, 3, 4, 5], - }, - 1 - ); - await flush(2); + await setDefaultData('0x01', 1); // force clear get cache (cacheModel as any).getCache.clear(); @@ -533,18 +537,13 @@ describe('cacheModel integration', () => { }); it('empty array test, compare db result with cache data', async () => { - cacheModel.set( - `0x09`, - { - id: `0x09`, - selfStake: BigInt(1000000000000000000000n), - oneEntity: {testItem: 'test', amount: BigInt(8000000000000000000000n)}, - delegators: [{delegator: '0x02', amount: BigInt(1000000000000000000000n)}], - randomNArray: undefined, - }, - 1 - ); - await flush(2); + await setDefaultData('0x09', 1, { + id: '0x09', + selfStake: BigInt(1000000000000000000000n), + oneEntity: {testItem: 'test', amount: BigInt(8000000000000000000000n)}, + delegators: [{delegator: '0x02', amount: BigInt(1000000000000000000000n)}], + randomNArray: undefined, + }); // Cache value 1, before cache is cleared const resCache1 = await cacheModel.get('0x09'); @@ -567,5 +566,22 @@ describe('cacheModel integration', () => { // We are expecting DB value and set cache value can be difference, field value can be undefined and null expect(res0).not.toEqual(resCache1); }); + + it('get and update, without save, get again should not be updated value', async () => { + await setDefaultData(`0x10`, 1); + + // Db value + const res0 = await cacheModel.get('0x10'); + const copiedRes0 = cloneDeep(res0); + + // Update the value + res0?.delegators.push({delegator: '0x11', amount: BigInt(9000000000000000000000n)}); + + // Get it again + const res1 = await cacheModel.get('0x10'); + + // should be same before updated + expect(res1).toEqual(copiedRes0); + }); }); }); diff --git a/packages/node-core/src/indexer/storeCache/cacheModel.ts b/packages/node-core/src/indexer/storeCache/cacheModel.ts index b2aa5120a8..c986ea5b48 100644 --- a/packages/node-core/src/indexer/storeCache/cacheModel.ts +++ b/packages/node-core/src/indexer/storeCache/cacheModel.ts @@ -45,7 +45,7 @@ export class CachedModel< T extends {id: string; __block_range?: (number | null)[] | Fn} = { id: string; __block_range?: (number | null)[] | Fn; - } + }, > extends Cacheable implements ICachedModel, ICachedModelControl @@ -93,7 +93,7 @@ export class CachedModel< !latestSetRecord.removed && this.removeCache[id].removedAtBlock <= latestSetRecord.startHeight ) { - return latestSetRecord.data; + return cloneDeep(latestSetRecord.data); } return; } @@ -117,7 +117,7 @@ export class CachedModel< this.getCache.set(id, record); } } - return record; + return cloneDeep(record); } return cloneDeep(this.getCache.get(id)); @@ -219,10 +219,11 @@ export class CachedModel< if (this.setCache[id] === undefined) { this.setCache[id] = new SetValueModel(); } - this.setCache[id].set(data, blockHeight, this.getNextStoreOperationIndex()); + const copiedData = cloneDeep(data); + this.setCache[id].set(copiedData, blockHeight, this.getNextStoreOperationIndex()); // Experimental, this means getCache keeps duplicate data from setCache, // we can remove this once memory is too full. - this.getCache.set(id, data); + this.getCache.set(id, copiedData); // Handle remove cache, when removed data been created again if (this.removeCache[id] && this.removeCache[id].removedAtBlock === blockHeight) { delete this.removeCache[id]; @@ -350,15 +351,22 @@ export class CachedModel< } } - private filterRecordsWithHeight(blockHeight: number): FilteredHeightRecords { - return { - removeRecords: Object.entries(this.removeCache).reduce((acc, [key, value]) => { - if (value.removedAtBlock <= blockHeight) { + private filterRemoveRecordByHeight(blockHeight: number, lessEqt: boolean): Record { + return Object.entries(this.removeCache).reduce( + (acc, [key, value]) => { + if (lessEqt ? value.removedAtBlock <= blockHeight : value.removedAtBlock > blockHeight) { acc[key] = value; } return acc; - }, {} as Record), + }, + {} as Record + ); + } + + private filterRecordsWithHeight(blockHeight: number): FilteredHeightRecords { + return { + removeRecords: this.filterRemoveRecordByHeight(blockHeight, true), setRecords: Object.entries(this.setCache).reduce((acc, [key, value]) => { const newValue = value.fromBelowHeight(blockHeight + 1); if (newValue.getValues().length) { @@ -422,13 +430,7 @@ export class CachedModel< return acc; }, {} as SetData); - this.removeCache = Object.entries(this.removeCache).reduce((acc, [key, value]) => { - if (value.removedAtBlock > blockHeight) { - newCounter++; - acc[key] = value; - } - return acc; - }, {} as Record); + this.removeCache = this.filterRemoveRecordByHeight(blockHeight, false); this.flushableRecordCounter = newCounter; } diff --git a/packages/node-core/src/utils/graphql.ts b/packages/node-core/src/utils/graphql.ts index 22988cb7fc..3e48958f2c 100644 --- a/packages/node-core/src/utils/graphql.ts +++ b/packages/node-core/src/utils/graphql.ts @@ -12,6 +12,7 @@ import { isBuffer, isNull, GraphQLEntityField, + GraphQLJsonFieldType, } from '@subql/utils'; import {ModelAttributes, ModelAttributeColumnOptions} from '@subql/x-sequelize'; import {isArray, isObject} from 'lodash'; @@ -120,7 +121,7 @@ export function getColumnOption( if (!dataValue || !field.jsonInterface) { return null; } - return field.isArray ? dataValue.map((v: any) => processGetJson(v)) : processGetJson(dataValue); + return field.isArray ? dataValue.map((v: any) => processGetJson(field, v)) : processGetJson(field, dataValue); }; columnOption.set = function (val: unknown) { if (val === undefined || isNull(val)) { @@ -140,14 +141,35 @@ export function getColumnOption( return columnOption; } -function processGetJson(data: any): any { - return JSON.parse(JSON.stringify(data), (key, value) => { - // regex to check if the value is a bigint string - if (typeof value === 'string' && /^-?\d+n$/.test(value)) { - return BigInt(value.slice(0, -1)); +/*** + * Process nested json get, output is same as input value type + * @param field + * @param value + */ +function processGetJson(field: GraphQLEntityField | GraphQLJsonFieldType, value: any): any { + // bigIntFields and nestJsonFields from this level in the entity/json + const bigIntFields = field.jsonInterface?.fields.filter((f) => f.type === 'BigInt'); + const nestJsonFields = field.jsonInterface?.fields.filter((f) => f.jsonInterface); + const processBigIntFields = (value: any) => { + if (bigIntFields) { + for (const bigIntField of bigIntFields) { + // If null is passed, we should not convert it to BigInt + if (value[bigIntField.name] !== undefined && value[bigIntField.name] !== null) { + value[bigIntField.name] = BigInt(value[bigIntField.name].slice(0, -1)); + } + } } return value; - }); + }; + if (nestJsonFields) { + for (const nestJsonField of nestJsonFields) { + // have a nest field, and nest field is json type, also value is defined + if (nestJsonField.jsonInterface && value[nestJsonField.name]) { + value[nestJsonField.name] = processGetJson(nestJsonField, value[nestJsonField.name]); + } + } + } + return processBigIntFields(value); } function processSetJson(data: any): any {