From 9033554abaa83156407b73b5c3534735c9a52c49 Mon Sep 17 00:00:00 2001 From: Amadeo Pellicce Date: Wed, 26 Oct 2022 16:40:44 +0200 Subject: [PATCH] refactoring asset and network schema rules to fix issue with submodule validation giving false negatives --- src/index.ts | 6 +- src/utils/json-schema.ts | 14 +- src/validate/rules/index.ts | 19 ++- .../rules/network/AssetSchemaRules.ts | 128 ++++++++---------- .../rules/network/NetworkDirectoryRules.ts | 5 +- .../rules/network/NetworkSchemaRules.ts | 16 +-- 6 files changed, 91 insertions(+), 97 deletions(-) diff --git a/src/index.ts b/src/index.ts index 8c1993c..377c73a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -24,9 +24,9 @@ export * from './tsv'; // .then(console.log) // .catch(console.error); -// validate('all', '/Users/ap/ama_dev/map3/indexer/tmp/map3xyz-assets/') -// .then(console.log) -// .catch(console.error); +validate('all', '/Users/ap/ama_dev/map3/indexer/tmp/map3xyz-assets/networks/smartchain/assets/smartchain-tokenlist') +.then(console.log) +.catch(console.error); // RepoFileGenerator.generate() diff --git a/src/utils/json-schema.ts b/src/utils/json-schema.ts index cd3c857..c4dd909 100644 --- a/src/utils/json-schema.ts +++ b/src/utils/json-schema.ts @@ -4,20 +4,28 @@ interface JsonSchemaValidationResponse { valid: boolean; errors: string[]; } +const v = new Validator(); + export function validateJsonSchema(schema: any, instance: any): JsonSchemaValidationResponse { try { - const v = new Validator(); + if(!schema || !instance) { + return { + valid: false, + errors: ['Schema or instance is null'] + } + } + const result = v.validate(instance, schema); return { - valid: result.valid, + valid: result?.valid, errors: result.errors.length > 0? result.errors.map(e => `${e.property} ${e.message}`): [] }; } catch (err) { return { - valid: true, + valid: false, errors: ['Unknown error while validating Json Schema' + err] }; } diff --git a/src/validate/rules/index.ts b/src/validate/rules/index.ts index c36053f..14d5dbc 100644 --- a/src/validate/rules/index.ts +++ b/src/validate/rules/index.ts @@ -1,8 +1,9 @@ import fs from 'fs'; import { getDirectories } from '../../utils/filesystem'; -import { AssetSchemaRules } from './network/AssetSchemaRules'; +import { AssetSchemaRules, fetchAssetSchema } from './network/AssetSchemaRules'; import { CoreFilesIntegrityRules, EditorPermissionRules, RepoStructureRules } from './core'; import { NetworkDirectoryRules, NetworkImagesRules, NetworkSchemaRules, NetworkSpecificRules, NetworkSubdirectoryRules } from './network'; +import { fetchNetworkSchema } from './network/NetworkSchemaRules'; export interface ValidationRule { name: string; @@ -65,6 +66,9 @@ async function validateRules(network: string, _rules: ValidationRule[], repoPath await Promise.all(_rules.map(rule => { return rule.validate(network, repoPath).then(result => { + if(!result) { + throw new Error(`Rule ${rule.name} returned null`); + } if (!result.valid) { valid = false; errors.push(...result.errors); @@ -72,7 +76,7 @@ async function validateRules(network: string, _rules: ValidationRule[], repoPath }).catch(err => { valid = false; errors.push({ - source: rule.name + ':' + rule.network, + source: rule.name + ':' + rule.network + ':' + repoPath, message: err.message }); }); @@ -97,6 +101,8 @@ function extractNetworkFromDir(dir: string): string { if(parts[parts.length - 2] === 'networks' || parts[parts.length - 2] === 'testnets') { return parts[parts.length -1]; + } else if (parts[parts.length - 2].endsWith('-tokenlist')) { + return parts[parts.length - 2].split("-")[0]; } else { return null; } @@ -120,8 +126,9 @@ async function traverseAndValidateNetworks(baseDir: string, rules: ValidationRul const split = subDir.split('/'); const isNetworkDir = split[split.length - 2] === 'networks' || split[split.length - 2] === 'testnets'; - - return !isSkipDir && isNetworkDir; + const isTokenListDir = split[split.length - 2].endsWith('-tokenlist'); + + return !isSkipDir && (isNetworkDir || isTokenListDir); }); return Promise.all(dirsToTraverse.map(async (subDir) => { @@ -172,6 +179,10 @@ export async function validate(network: string = 'all', repoPath: string): Promi const errors = []; let valid = true; + // warm up cache + await fetchAssetSchema(); + await fetchNetworkSchema(); + const coreRulesResult = await validateRules(network, coreRules, repoPath); const nextworkRulesResult = await validateNetworkRules(network, networkRules, repoPath); diff --git a/src/validate/rules/network/AssetSchemaRules.ts b/src/validate/rules/network/AssetSchemaRules.ts index 6106408..73715e8 100644 --- a/src/validate/rules/network/AssetSchemaRules.ts +++ b/src/validate/rules/network/AssetSchemaRules.ts @@ -4,7 +4,6 @@ import fs from "fs"; import path from "path"; import { validateJsonSchema } from "../../../utils/json-schema"; import { ASSETS_SCHEMA_FILE_URL } from "../../../utils/constants"; -import { getDirectories } from "../../../utils"; const baseName = 'AssetSchemaRules'; let assetsSchema; @@ -15,10 +14,9 @@ export async function fetchAssetSchema(): Promise { } try { - assetsSchema = (await axios.get(ASSETS_SCHEMA_FILE_URL)).data; + assetsSchema = (await axios.get(ASSETS_SCHEMA_FILE_URL))?.data; } catch (err) { - console.error('fetchAssetSchema', err); - throw err; + throw new Error('Unable to fetch assets schema file. ' + err.message); } return Promise.resolve(assetsSchema); @@ -30,90 +28,80 @@ export const AssetSchemaRules: ValidationRule[] = [ validate: async (network: string, repoPath: string): Promise => { try { - const networkHasAssets = fs.existsSync(path.join(repoPath, 'assets')); - - if(!networkHasAssets) { + const split = repoPath.split('/'); + const assetDirName = split[split.length - 1]; + const isAssetDir = split[split.length - 2].endsWith('-tokenlist'); + + if(!isAssetDir) { return { valid: true, errors: []}; } - + let valid = true; const errors = []; + + const infoFilePath = path.join(repoPath, 'info.json'); + const infoFileExists = fs.existsSync(infoFilePath); + + if(!infoFileExists) { + valid = false; + errors.push({ + source: `${baseName}:InfoFilesAreInstanceOfSchema ${repoPath}`, + message: `Info.json is missing` + }); + return { valid: valid, errors: errors}; + } await fetchAssetSchema(); - const assetDirs = (await getDirectories(path.join(repoPath, 'assets', `${network}-tokenlist`))) - .filter(dir => !dir.includes('.git')); - - assetDirs.forEach(asset => { + const infoFile = JSON.parse(fs.readFileSync(infoFilePath, 'utf-8')); - if(asset.endsWith('-tokenlist')) { - return; // skip the tokenlist directory - } - - const infoFilePath = path.join(asset, 'info.json'); - const infoFileExists = fs.existsSync(infoFilePath); - - if(!infoFileExists) { - valid = false; - errors.push({ - source: `${baseName}:InfoFilesAreInstanceOfSchema ${asset}`, - message: `${asset} info.json is missing` - }); - return; - } - - const infoFile = JSON.parse(fs.readFileSync(infoFilePath, 'utf-8')); - - const result = infoFile.type = 'asset' ? - validateJsonSchema(infoFile, assetsSchema) : - { valid: true, errors: []}; - - if(!result.valid) { - valid = false; - errors.push(result.errors.map(e => { - return { - source: `${baseName}:InfoFilesAreInstanceOfSchema ${asset}`, - message: e - } - })); - } + const result = validateJsonSchema(infoFile, assetsSchema); - if(infoFile.logo) { - if(infoFile.logo.png) { - const pngFilePath = path.join(asset, 'logo.png'); - const pngFileExists = fs.existsSync(pngFilePath); - - if(!pngFileExists) { - valid = false; - errors.push({ - source: `${baseName}:InfoFilesAreInstanceOfSchema ${asset}`, - message: `${asset} png logo is missing` - }); - } + if(!result.valid) { + valid = false; + errors.push(result.errors.map(e => { + return { + source: `${baseName}:InfoFilesAreInstanceOfSchema ${repoPath}`, + message: e } + })); + } + + if(infoFile.logo) { + if(infoFile.logo.png) { + const pngFilePath = path.join(repoPath, 'logo.png'); + const pngFileExists = fs.existsSync(pngFilePath); - if(infoFile.logo.svg) { - const svgFilePath = path.join(asset, 'logo.svg'); - const svgFileExists = fs.existsSync(svgFilePath); - - if(!svgFileExists) { - valid = false; - errors.push({ - source: `${baseName}:InfoFilesAreInstanceOfSchema ${asset}`, - message: `${asset} svg logo is missing` - }); - } + if(!pngFileExists) { + valid = false; + errors.push({ + source: `${baseName}:InfoFilesAreInstanceOfSchema ${repoPath}`, + message: `Png logo is declared but missing ${pngFilePath}` + }); } + } + + if(infoFile.logo.svg) { + const svgFilePath = path.join(repoPath, 'logo.svg'); + const svgFileExists = fs.existsSync(svgFilePath); - if(!asset.endsWith(infoFile.address)) { + if(!svgFileExists) { valid = false; errors.push({ - source: `${baseName}:InfoFilesAreInstanceOfSchema ${asset}`, - message: `${asset} address ${infoFile.address} does not match directory name` + source: `${baseName}:InfoFilesAreInstanceOfSchema ${repoPath}`, + message: `Svg logo is declared but missing ${svgFilePath}` }); } } - }); + } + + if(!assetDirName.endsWith(infoFile.address)) { + valid = false; + errors.push({ + source: `${baseName}:InfoFilesAreInstanceOfSchema ${repoPath}`, + message: `Info.json Address ${infoFile.address} does not match directory name` + }); + } return { valid: valid, diff --git a/src/validate/rules/network/NetworkDirectoryRules.ts b/src/validate/rules/network/NetworkDirectoryRules.ts index d82db1c..3fc419e 100644 --- a/src/validate/rules/network/NetworkDirectoryRules.ts +++ b/src/validate/rules/network/NetworkDirectoryRules.ts @@ -17,8 +17,9 @@ export const NetworkDirectoryRules: ValidationRule[] = [ return new Promise((resolve, reject) => { - - if(!network) { + const split = repoPath.split('/'); + + if(!network || split[split.length - 2] !== 'networks') { return resolve({valid: true, errors: []}) } diff --git a/src/validate/rules/network/NetworkSchemaRules.ts b/src/validate/rules/network/NetworkSchemaRules.ts index 95603b0..5e4ab27 100644 --- a/src/validate/rules/network/NetworkSchemaRules.ts +++ b/src/validate/rules/network/NetworkSchemaRules.ts @@ -26,22 +26,8 @@ export const NetworkSchemaRules: ValidationRule[] = [ name: `${baseName}:InfoFilesAreInstanceOfSchema`, network: 'all', validate: async (network: string, repoPath: string): Promise => { - - let directoryShouldHaveInfoFile = false; - // Note: this relies on the fact that in the case of - // tokens such as ethereum/assets/{usdc_contract_address} - // the directory structure contains a '/' - // before the variable token or testnet id/name - - if(repoPath.endsWith(network) - || repoPath.includes(`${network}/assets/`) - || repoPath.includes(`${network}/testnets/`) - ) { - directoryShouldHaveInfoFile = true; - } - - if(!directoryShouldHaveInfoFile) { + if(!repoPath.endsWith(network)) { return { valid: true, errors: []}; }