From f7624a737208fe4fc00802c4f60fc03995052109 Mon Sep 17 00:00:00 2001 From: valadaptive Date: Sun, 14 Jan 2024 06:03:58 -0500 Subject: [PATCH 1/9] Pass import hook params as an object --- doc | 2 +- etc/browser/lib/files.js | 2 +- lib/files.js | 4 ++-- lib/specs.js | 4 ++-- test/test_specs.js | 6 +++--- types/index.d.ts | 8 +++++++- 6 files changed, 16 insertions(+), 10 deletions(-) diff --git a/doc b/doc index 4e0d8af9..99654b49 160000 --- a/doc +++ b/doc @@ -1 +1 @@ -Subproject commit 4e0d8af96093cd5502e0b8afc7b29b0c16ff2dda +Subproject commit 99654b49656f79d415f6504f7f238a50ad479ff8 diff --git a/etc/browser/lib/files.js b/etc/browser/lib/files.js index f407d227..a170467d 100644 --- a/etc/browser/lib/files.js +++ b/etc/browser/lib/files.js @@ -5,7 +5,7 @@ function createError() { return new Error('unsupported in the browser'); } function createImportHook() { - return function (fpath, kind, cb) { cb(createError()); }; + return function (_, cb) { cb(createError()); }; } function createSyncImportHook() { diff --git a/lib/files.js b/lib/files.js index bdfd85ac..4e36e064 100644 --- a/lib/files.js +++ b/lib/files.js @@ -13,7 +13,7 @@ let fs = require('fs'), /** Default (asynchronous) file loading function for assembling IDLs. */ function createImportHook() { let imports = {}; - return function (fpath, kind, cb) { + return function ({path: fpath}, cb) { fpath = path.resolve(fpath); if (imports[fpath]) { // Already imported, return nothing to avoid duplicating attributes. @@ -35,7 +35,7 @@ function createImportHook() { */ function createSyncImportHook() { let imports = {}; - return function (fpath, kind, cb) { + return function ({path: fpath}, cb) { fpath = path.resolve(fpath); if (imports[fpath]) { cb(); diff --git a/lib/specs.js b/lib/specs.js index 88c75189..3ed69434 100644 --- a/lib/specs.js +++ b/lib/specs.js @@ -58,7 +58,7 @@ function assembleProtocol(fpath, opts, cb) { }); function importFile(fpath, cb) { - opts.importHook(fpath, 'idl', (err, str) => { + opts.importHook({path: fpath, kind: 'idl'}, (err, str) => { if (err) { cb(err); return; @@ -118,7 +118,7 @@ function assembleProtocol(fpath, opts, cb) { }); } else { // We are importing a protocol or schema file. - opts.importHook(importPath, info.kind, (err, str) => { + opts.importHook({path: importPath, kind: info.kind}, (err, str) => { if (err) { cb(err); return; diff --git a/test/test_specs.js b/test/test_specs.js index 1ee95708..74629ecf 100644 --- a/test/test_specs.js +++ b/test/test_specs.js @@ -449,7 +449,7 @@ suite('specs', () => { }); test('import hook error', (done) => { - let hook = function (fpath, kind, cb) { + let hook = function ({path: fpath}, cb) { if (path.basename(fpath) === 'A.avdl') { cb(null, 'import schema "hi"; protocol A {}'); } else { @@ -463,7 +463,7 @@ suite('specs', () => { }); test('import hook idl error', (done) => { - let hook = function (fpath, kind, cb) { + let hook = function ({path: fpath}, cb) { if (path.basename(fpath) === 'A.avdl') { cb(null, 'import idl "hi"; protocol A {}'); } else { @@ -624,7 +624,7 @@ suite('specs', () => { // Import hook from strings. function createImportHook(imports) { - return function (fpath, kind, cb) { + return function ({path: fpath}, cb) { let key = path.normalize(fpath); let str = imports[key]; delete imports[key]; diff --git a/types/index.d.ts b/types/index.d.ts index 224a0922..b9f87c90 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -127,8 +127,14 @@ interface IsValidOptions { noUndeclaredFields: boolean; errorHook: (path: string[], val: any, type: Type) => void } + +interface ImportHookParams { + path: string; + type: 'idl' | 'protocol' | 'schema'; +} + interface AssembleOptions { - importHook: (filePath: string, type: 'idl', callback: Callback) => void; + importHook: (params: ImportHookParams, callback: Callback) => void; } interface SchemaOptions { From 03391e5d5b01e5b45aac7f4f2751ea40a2f05906 Mon Sep 17 00:00:00 2001 From: valadaptive Date: Sun, 14 Jan 2024 07:25:12 -0500 Subject: [PATCH 2/9] Update import hook to provide path to callback This lets us move all path resolution into the import hook, making it independent of Node's `fs` module. --- doc | 2 +- lib/files.js | 18 ++++++++++++------ lib/specs.js | 28 ++++++++++++++++------------ test/test_specs.js | 19 ++++++++++++++----- types/index.d.ts | 4 ++++ 5 files changed, 47 insertions(+), 24 deletions(-) diff --git a/doc b/doc index 99654b49..63721391 160000 --- a/doc +++ b/doc @@ -1 +1 @@ -Subproject commit 99654b49656f79d415f6504f7f238a50ad479ff8 +Subproject commit 63721391214220cb9a858be23024235930165ede diff --git a/lib/files.js b/lib/files.js index 4e36e064..d1cec04c 100644 --- a/lib/files.js +++ b/lib/files.js @@ -13,15 +13,18 @@ let fs = require('fs'), /** Default (asynchronous) file loading function for assembling IDLs. */ function createImportHook() { let imports = {}; - return function ({path: fpath}, cb) { - fpath = path.resolve(fpath); + return function ({path: fpath, parentPath}, cb) { + fpath = path.resolve(path.dirname(parentPath), fpath); if (imports[fpath]) { // Already imported, return nothing to avoid duplicating attributes. process.nextTick(cb); return; } imports[fpath] = true; - fs.readFile(fpath, {encoding: 'utf8'}, cb); + fs.readFile(fpath, {encoding: 'utf8'}, (err, data) => { + if (err) return cb(err); + return cb(null, {contents: data, path: fpath}); + }); }; } @@ -35,13 +38,16 @@ function createImportHook() { */ function createSyncImportHook() { let imports = {}; - return function ({path: fpath}, cb) { - fpath = path.resolve(fpath); + return function ({path: fpath, parentPath}, cb) { + fpath = path.resolve(path.dirname(parentPath), fpath); if (imports[fpath]) { cb(); } else { imports[fpath] = true; - cb(null, fs.readFileSync(fpath, {encoding: 'utf8'})); + cb(null, { + contents: fs.readFileSync(fpath, {encoding: 'utf8'}), + path: fpath + }); } }; } diff --git a/lib/specs.js b/lib/specs.js index 3ed69434..8036a093 100644 --- a/lib/specs.js +++ b/lib/specs.js @@ -31,7 +31,7 @@ function assembleProtocol(fpath, opts, cb) { opts.importHook = files.createImportHook(); } - importFile(fpath, (err, protocol) => { + importFile(fpath, '', (err, protocol) => { if (err) { cb(err); return; @@ -57,19 +57,20 @@ function assembleProtocol(fpath, opts, cb) { cb(null, protocol); }); - function importFile(fpath, cb) { - opts.importHook({path: fpath, kind: 'idl'}, (err, str) => { + function importFile(fpath, parentPath, cb) { + opts.importHook({path: fpath, parentPath, kind: 'idl'}, (err, params) => { if (err) { cb(err); return; } - if (str === undefined) { + if (!params) { // This signals an already imported file by the default import hooks. // Implementors who wish to disallow duplicate imports should provide a // custom hook which throws an error when a duplicate is detected. cb(); return; } + const {contents: str, path: fpath} = params; let obj; try { let reader = new Reader(str, opts); @@ -79,11 +80,11 @@ function assembleProtocol(fpath, opts, cb) { cb(err); return; } - fetchImports(obj.protocol, obj.imports, path.dirname(fpath), cb); + fetchImports(obj.protocol, obj.imports, fpath, cb); }); } - function fetchImports(protocol, imports, dpath, cb) { + function fetchImports(protocol, imports, fpath, cb) { let importedProtocols = []; next(); @@ -104,9 +105,8 @@ function assembleProtocol(fpath, opts, cb) { cb(null, protocol); return; } - let importPath = path.join(dpath, info.name); if (info.kind === 'idl') { - importFile(importPath, (err, imported) => { + importFile(info.name, fpath, (err, imported) => { if (err) { cb(err); return; @@ -118,7 +118,11 @@ function assembleProtocol(fpath, opts, cb) { }); } else { // We are importing a protocol or schema file. - opts.importHook({path: importPath, kind: info.kind}, (err, str) => { + opts.importHook({ + path: info.name, + parentPath: fpath, + kind: info.kind + }, (err, params) => { if (err) { cb(err); return; @@ -126,16 +130,16 @@ function assembleProtocol(fpath, opts, cb) { switch (info.kind) { case 'protocol': case 'schema': { - if (str === undefined) { + if (!params) { // Skip duplicate import (see related comment above). next(); return; } let obj; try { - obj = JSON.parse(str); + obj = JSON.parse(params.contents); } catch (err) { - err.path = importPath; + err.path = params.path; cb(err); return; } diff --git a/test/test_specs.js b/test/test_specs.js index 74629ecf..d44d82fb 100644 --- a/test/test_specs.js +++ b/test/test_specs.js @@ -451,7 +451,10 @@ suite('specs', () => { test('import hook error', (done) => { let hook = function ({path: fpath}, cb) { if (path.basename(fpath) === 'A.avdl') { - cb(null, 'import schema "hi"; protocol A {}'); + cb(null, { + contents: 'import schema "hi"; protocol A {}', + path: fpath + }); } else { cb(new Error('foo')); } @@ -465,7 +468,10 @@ suite('specs', () => { test('import hook idl error', (done) => { let hook = function ({path: fpath}, cb) { if (path.basename(fpath) === 'A.avdl') { - cb(null, 'import idl "hi"; protocol A {}'); + cb(null, { + contents: 'import idl "hi"; protocol A {}', + path: fpath + }); } else { cb(new Error('bar')); } @@ -624,11 +630,14 @@ suite('specs', () => { // Import hook from strings. function createImportHook(imports) { - return function ({path: fpath}, cb) { - let key = path.normalize(fpath); + return function ({path: fpath, parentPath}, cb) { + let key = path.normalize(path.join(path.dirname(parentPath), fpath)); let str = imports[key]; delete imports[key]; - process.nextTick(() => { cb(null, str); }); + process.nextTick(() => { cb(null, typeof str === 'string' ? { + contents: str, + path: key + } : undefined); }); }; } diff --git a/types/index.d.ts b/types/index.d.ts index b9f87c90..c7727b3c 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -133,6 +133,10 @@ interface ImportHookParams { type: 'idl' | 'protocol' | 'schema'; } +type ImportHookCallback = (err: any, params?: {contents: string, path: string}) => void; + +type ImportHook = (params: ImportHookParams, cb: ImportHookCallback) => void; + interface AssembleOptions { importHook: (params: ImportHookParams, callback: Callback) => void; } From d306ba4dbf78f005bedfed75ecaed8b0aa2351c1 Mon Sep 17 00:00:00 2001 From: valadaptive Date: Sun, 14 Jan 2024 07:29:14 -0500 Subject: [PATCH 3/9] Remove last `path` usage from specs This does sacrifice the ability to use Windows-style paths, but it's a convenience function that does "magic" stuff to its argument anyways, so I don't see it as much of a loss. --- lib/specs.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/specs.js b/lib/specs.js index 8036a093..882f83c8 100644 --- a/lib/specs.js +++ b/lib/specs.js @@ -7,8 +7,7 @@ /** IDL to protocol (services) and schema (types) parsing logic. */ let files = require('./files'), - utils = require('./utils'), - path = require('path'); + utils = require('./utils'); // Default type references defined by Avro. @@ -197,7 +196,7 @@ function assembleProtocol(fpath, opts, cb) { * * The parsing logic is as follows: * - * + If `str` contains `path.sep` (on windows `\`, otherwise `/`) and is a path + * + If `str` contains `/` and is a path * to an existing file, it will first be read as JSON, then as an IDL * specification if JSON parsing failed. If either succeeds, the result is * returned, otherwise the next steps are run using the file's content @@ -213,7 +212,7 @@ function read(str) { let schema; if ( typeof str == 'string' && - ~str.indexOf(path.sep) && + str.indexOf('/') !== -1 && files.existsSync(str) ) { // Try interpreting `str` as path to a file contain a JSON schema or an IDL From de18aa2d8e28d685cd17742a0535788f4a502ea5 Mon Sep 17 00:00:00 2001 From: valadaptive Date: Sat, 20 Jan 2024 13:22:07 -0500 Subject: [PATCH 4/9] Rename ImportHookParams to ImportHookPayload --- lib/specs.js | 14 +++++++------- types/index.d.ts | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/specs.js b/lib/specs.js index 882f83c8..7320db6c 100644 --- a/lib/specs.js +++ b/lib/specs.js @@ -57,19 +57,19 @@ function assembleProtocol(fpath, opts, cb) { }); function importFile(fpath, parentPath, cb) { - opts.importHook({path: fpath, parentPath, kind: 'idl'}, (err, params) => { + opts.importHook({path: fpath, parentPath, kind: 'idl'}, (err, payload) => { if (err) { cb(err); return; } - if (!params) { + if (!payload) { // This signals an already imported file by the default import hooks. // Implementors who wish to disallow duplicate imports should provide a // custom hook which throws an error when a duplicate is detected. cb(); return; } - const {contents: str, path: fpath} = params; + const {contents: str, path: fpath} = payload; let obj; try { let reader = new Reader(str, opts); @@ -121,7 +121,7 @@ function assembleProtocol(fpath, opts, cb) { path: info.name, parentPath: fpath, kind: info.kind - }, (err, params) => { + }, (err, payload) => { if (err) { cb(err); return; @@ -129,16 +129,16 @@ function assembleProtocol(fpath, opts, cb) { switch (info.kind) { case 'protocol': case 'schema': { - if (!params) { + if (!payload) { // Skip duplicate import (see related comment above). next(); return; } let obj; try { - obj = JSON.parse(params.contents); + obj = JSON.parse(payload.contents); } catch (err) { - err.path = params.path; + err.path = payload.path; cb(err); return; } diff --git a/types/index.d.ts b/types/index.d.ts index c7727b3c..63d46c48 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -128,17 +128,17 @@ interface IsValidOptions { errorHook: (path: string[], val: any, type: Type) => void } -interface ImportHookParams { +interface ImportHookPayload { path: string; type: 'idl' | 'protocol' | 'schema'; } type ImportHookCallback = (err: any, params?: {contents: string, path: string}) => void; -type ImportHook = (params: ImportHookParams, cb: ImportHookCallback) => void; +type ImportHook = (payload: ImportHookPayload, cb: ImportHookCallback) => void; interface AssembleOptions { - importHook: (params: ImportHookParams, callback: Callback) => void; + importHook: (params: ImportHookPayload, callback: Callback) => void; } interface SchemaOptions { From 066d526d7426cb0edbbccd349297455a60a4b408 Mon Sep 17 00:00:00 2001 From: valadaptive Date: Sat, 20 Jan 2024 13:22:48 -0500 Subject: [PATCH 5/9] Rename parentPath to importerPath --- doc | 2 +- lib/files.js | 8 ++++---- lib/specs.js | 6 +++--- test/test_specs.js | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/doc b/doc index 63721391..5ddd9df9 160000 --- a/doc +++ b/doc @@ -1 +1 @@ -Subproject commit 63721391214220cb9a858be23024235930165ede +Subproject commit 5ddd9df9cff57d5d61fde6dd39f13f74e1f0a590 diff --git a/lib/files.js b/lib/files.js index d1cec04c..0ba09d16 100644 --- a/lib/files.js +++ b/lib/files.js @@ -13,8 +13,8 @@ let fs = require('fs'), /** Default (asynchronous) file loading function for assembling IDLs. */ function createImportHook() { let imports = {}; - return function ({path: fpath, parentPath}, cb) { - fpath = path.resolve(path.dirname(parentPath), fpath); + return function ({path: fpath, importerPath}, cb) { + fpath = path.resolve(path.dirname(importerPath), fpath); if (imports[fpath]) { // Already imported, return nothing to avoid duplicating attributes. process.nextTick(cb); @@ -38,8 +38,8 @@ function createImportHook() { */ function createSyncImportHook() { let imports = {}; - return function ({path: fpath, parentPath}, cb) { - fpath = path.resolve(path.dirname(parentPath), fpath); + return function ({path: fpath, importerPath}, cb) { + fpath = path.resolve(path.dirname(importerPath), fpath); if (imports[fpath]) { cb(); } else { diff --git a/lib/specs.js b/lib/specs.js index 7320db6c..186a28e0 100644 --- a/lib/specs.js +++ b/lib/specs.js @@ -56,8 +56,8 @@ function assembleProtocol(fpath, opts, cb) { cb(null, protocol); }); - function importFile(fpath, parentPath, cb) { - opts.importHook({path: fpath, parentPath, kind: 'idl'}, (err, payload) => { + function importFile(fpath, importerPath, cb) { + opts.importHook({path: fpath, importerPath, kind: 'idl'}, (err, payload) => { if (err) { cb(err); return; @@ -119,7 +119,7 @@ function assembleProtocol(fpath, opts, cb) { // We are importing a protocol or schema file. opts.importHook({ path: info.name, - parentPath: fpath, + importerPath: fpath, kind: info.kind }, (err, payload) => { if (err) { diff --git a/test/test_specs.js b/test/test_specs.js index d44d82fb..d2ce3118 100644 --- a/test/test_specs.js +++ b/test/test_specs.js @@ -630,8 +630,8 @@ suite('specs', () => { // Import hook from strings. function createImportHook(imports) { - return function ({path: fpath, parentPath}, cb) { - let key = path.normalize(path.join(path.dirname(parentPath), fpath)); + return function ({path: fpath, importerPath}, cb) { + let key = path.normalize(path.join(path.dirname(importerPath), fpath)); let str = imports[key]; delete imports[key]; process.nextTick(() => { cb(null, typeof str === 'string' ? { From d0ad9c3aec7c159d40206cbfba26c031638b29da Mon Sep 17 00:00:00 2001 From: valadaptive Date: Fri, 2 Feb 2024 21:07:29 -0500 Subject: [PATCH 6/9] Move `read` convenience function to files.js --- doc | 2 +- etc/browser/avsc-services.js | 3 +- etc/browser/lib/files.js | 4 +-- lib/files.js | 70 ++++++++++++++++++++++++++++++++++-- 4 files changed, 73 insertions(+), 6 deletions(-) diff --git a/doc b/doc index 5ddd9df9..bebd0480 160000 --- a/doc +++ b/doc @@ -1 +1 @@ -Subproject commit 5ddd9df9cff57d5d61fde6dd39f13f74e1f0a590 +Subproject commit bebd0480b9201978ee6c0f8eb787bdd8325e1b5e diff --git a/etc/browser/avsc-services.js b/etc/browser/avsc-services.js index e2a02d89..a73717a2 100644 --- a/etc/browser/avsc-services.js +++ b/etc/browser/avsc-services.js @@ -7,6 +7,7 @@ */ let avroTypes = require('./avsc-types'), + files = require('../../lib/files'), services = require('../../lib/services'), specs = require('../../lib/specs'), utils = require('../../lib/utils'); @@ -14,7 +15,7 @@ let avroTypes = require('./avsc-types'), /** Slightly enhanced parsing, supporting IDL declarations. */ function parse(any, opts) { - let schemaOrProtocol = specs.read(any); + let schemaOrProtocol = files.readSchemaFromPathOrString(any); return schemaOrProtocol.protocol ? services.Service.forProtocol(schemaOrProtocol, opts) : avroTypes.Type.forSchema(schemaOrProtocol, opts); diff --git a/etc/browser/lib/files.js b/etc/browser/lib/files.js index a170467d..7cbb99f8 100644 --- a/etc/browser/lib/files.js +++ b/etc/browser/lib/files.js @@ -12,10 +12,10 @@ function createSyncImportHook() { return function () { throw createError(); }; } - module.exports = { createImportHook, createSyncImportHook, existsSync: function () { return false; }, - readFileSync: function () { throw createError(); } + readFileSync: function () { throw createError(); }, + readSchemaFromPathOrString: function () { throw createError(); } }; diff --git a/lib/files.js b/lib/files.js index 0ba09d16..4500e8df 100644 --- a/lib/files.js +++ b/lib/files.js @@ -8,7 +8,8 @@ */ let fs = require('fs'), - path = require('path'); + path = require('path'), + specs = require('./specs'); /** Default (asynchronous) file loading function for assembling IDLs. */ function createImportHook() { @@ -52,11 +53,76 @@ function createSyncImportHook() { }; } +/** + * Convenience function to parse multiple inputs into protocols and schemas. + * + * It should cover most basic use-cases but has a few limitations: + * + * + It doesn't allow passing options to the parsing step. + * + The protocol/type inference logic can be deceived. + * + * The parsing logic is as follows: + * + * + If `str` contains `path.sep` (on windows `\`, otherwise `/`) and is a path + * to an existing file, it will first be read as JSON, then as an IDL + * specification if JSON parsing failed. If either succeeds, the result is + * returned, otherwise the next steps are run using the file's content + * instead of the input path. + * + If `str` is a valid JSON string, it is parsed then returned. + * + If `str` is a valid IDL protocol specification, it is parsed and returned + * if no imports are present (and an error is thrown if there are any + * imports). + * + If `str` is a valid IDL type specification, it is parsed and returned. + * + If neither of the above cases apply, `str` is returned. + */ +function readSchemaFromPathOrString(str) { + let schema; + if ( + typeof str == 'string' && + str.indexOf('/') !== -1 && + fs.existsSync(str) + ) { + // Try interpreting `str` as path to a file contain a JSON schema or an IDL + // protocol. Note that we add the second check to skip primitive references + // (e.g. `"int"`, the most common use-case for `avro.parse`). + let contents = fs.readFileSync(str, {encoding: 'utf8'}); + try { + return JSON.parse(contents); + } catch (err) { + let opts = {importHook: createSyncImportHook()}; + specs.assembleProtocol(str, opts, (err, protocolSchema) => { + schema = err ? contents : protocolSchema; + }); + } + } else { + schema = str; + } + if (typeof schema != 'string' || schema === 'null') { + // This last predicate is to allow `read('null')` to work similarly to + // `read('int')` and other primitives (null needs to be handled separately + // since it is also a valid JSON identifier). + return schema; + } + try { + return JSON.parse(schema); + } catch (err) { + try { + return specs.readProtocol(schema); + } catch (err) { + try { + return specs.readSchema(schema); + } catch (err) { + return schema; + } + } + } +} module.exports = { createImportHook, createSyncImportHook, // Proxy a few methods to better shim them for browserify. existsSync: fs.existsSync, - readFileSync: fs.readFileSync + readFileSync: fs.readFileSync, + readSchemaFromPathOrString }; From a6689a69f1947a2f9e26f52ce2cdb5f5d7c82d75 Mon Sep 17 00:00:00 2001 From: valadaptive Date: Wed, 21 Feb 2024 23:43:42 -0500 Subject: [PATCH 7/9] Abstract into tryReadFileSync --- etc/browser/avsc-services.js | 3 +- etc/browser/lib/files.js | 4 ++- lib/files.js | 69 ++++++------------------------------ lib/specs.js | 16 +++------ 4 files changed, 19 insertions(+), 73 deletions(-) diff --git a/etc/browser/avsc-services.js b/etc/browser/avsc-services.js index a73717a2..e2a02d89 100644 --- a/etc/browser/avsc-services.js +++ b/etc/browser/avsc-services.js @@ -7,7 +7,6 @@ */ let avroTypes = require('./avsc-types'), - files = require('../../lib/files'), services = require('../../lib/services'), specs = require('../../lib/specs'), utils = require('../../lib/utils'); @@ -15,7 +14,7 @@ let avroTypes = require('./avsc-types'), /** Slightly enhanced parsing, supporting IDL declarations. */ function parse(any, opts) { - let schemaOrProtocol = files.readSchemaFromPathOrString(any); + let schemaOrProtocol = specs.read(any); return schemaOrProtocol.protocol ? services.Service.forProtocol(schemaOrProtocol, opts) : avroTypes.Type.forSchema(schemaOrProtocol, opts); diff --git a/etc/browser/lib/files.js b/etc/browser/lib/files.js index 7cbb99f8..b984631f 100644 --- a/etc/browser/lib/files.js +++ b/etc/browser/lib/files.js @@ -12,10 +12,12 @@ function createSyncImportHook() { return function () { throw createError(); }; } +function tryReadFileSync() { return null; } + module.exports = { createImportHook, createSyncImportHook, existsSync: function () { return false; }, readFileSync: function () { throw createError(); }, - readSchemaFromPathOrString: function () { throw createError(); } + tryReadFileSync, }; diff --git a/lib/files.js b/lib/files.js index 4500e8df..a14a03ff 100644 --- a/lib/files.js +++ b/lib/files.js @@ -8,8 +8,7 @@ */ let fs = require('fs'), - path = require('path'), - specs = require('./specs'); + path = require('path'); /** Default (asynchronous) file loading function for assembling IDLs. */ function createImportHook() { @@ -54,68 +53,20 @@ function createSyncImportHook() { } /** - * Convenience function to parse multiple inputs into protocols and schemas. - * - * It should cover most basic use-cases but has a few limitations: - * - * + It doesn't allow passing options to the parsing step. - * + The protocol/type inference logic can be deceived. - * - * The parsing logic is as follows: - * - * + If `str` contains `path.sep` (on windows `\`, otherwise `/`) and is a path - * to an existing file, it will first be read as JSON, then as an IDL - * specification if JSON parsing failed. If either succeeds, the result is - * returned, otherwise the next steps are run using the file's content - * instead of the input path. - * + If `str` is a valid JSON string, it is parsed then returned. - * + If `str` is a valid IDL protocol specification, it is parsed and returned - * if no imports are present (and an error is thrown if there are any - * imports). - * + If `str` is a valid IDL type specification, it is parsed and returned. - * + If neither of the above cases apply, `str` is returned. + * Check if the given input string is "path-like" or a path to an existing file, + * and if so, read it. This requires it to contain a path separator + * (`path.sep`). If not, this will return `null`. */ -function readSchemaFromPathOrString(str) { - let schema; +function tryReadFileSync(str) { if ( typeof str == 'string' && - str.indexOf('/') !== -1 && + str.indexOf(path.sep) !== -1 && fs.existsSync(str) ) { - // Try interpreting `str` as path to a file contain a JSON schema or an IDL - // protocol. Note that we add the second check to skip primitive references - // (e.g. `"int"`, the most common use-case for `avro.parse`). - let contents = fs.readFileSync(str, {encoding: 'utf8'}); - try { - return JSON.parse(contents); - } catch (err) { - let opts = {importHook: createSyncImportHook()}; - specs.assembleProtocol(str, opts, (err, protocolSchema) => { - schema = err ? contents : protocolSchema; - }); - } - } else { - schema = str; - } - if (typeof schema != 'string' || schema === 'null') { - // This last predicate is to allow `read('null')` to work similarly to - // `read('int')` and other primitives (null needs to be handled separately - // since it is also a valid JSON identifier). - return schema; - } - try { - return JSON.parse(schema); - } catch (err) { - try { - return specs.readProtocol(schema); - } catch (err) { - try { - return specs.readSchema(schema); - } catch (err) { - return schema; - } - } + // Try interpreting `str` as path to a file. + return fs.readFileSync(str, {encoding: 'utf8'}); } + return null; } module.exports = { @@ -124,5 +75,5 @@ module.exports = { // Proxy a few methods to better shim them for browserify. existsSync: fs.existsSync, readFileSync: fs.readFileSync, - readSchemaFromPathOrString + tryReadFileSync }; diff --git a/lib/specs.js b/lib/specs.js index 186a28e0..45e753b3 100644 --- a/lib/specs.js +++ b/lib/specs.js @@ -210,15 +210,11 @@ function assembleProtocol(fpath, opts, cb) { */ function read(str) { let schema; - if ( - typeof str == 'string' && - str.indexOf('/') !== -1 && - files.existsSync(str) - ) { - // Try interpreting `str` as path to a file contain a JSON schema or an IDL - // protocol. Note that we add the second check to skip primitive references - // (e.g. `"int"`, the most common use-case for `avro.parse`). - let contents = files.readFileSync(str, {encoding: 'utf8'}); + let contents = files.tryReadFileSync(str); + + if (contents === null) { + schema = str; + } else { try { return JSON.parse(contents); } catch (err) { @@ -227,8 +223,6 @@ function read(str) { schema = err ? contents : protocolSchema; }); } - } else { - schema = str; } if (typeof schema != 'string' || schema === 'null') { // This last predicate is to allow `read('null')` to work similarly to From 59b5035851203ba8f3697f44c23affcd66e591a9 Mon Sep 17 00:00:00 2001 From: valadaptive Date: Thu, 22 Feb 2024 00:50:31 -0500 Subject: [PATCH 8/9] Seek ENOENTness rather than asking existsSync --- lib/files.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/files.js b/lib/files.js index a14a03ff..b9405f46 100644 --- a/lib/files.js +++ b/lib/files.js @@ -60,11 +60,15 @@ function createSyncImportHook() { function tryReadFileSync(str) { if ( typeof str == 'string' && - str.indexOf(path.sep) !== -1 && - fs.existsSync(str) + str.indexOf(path.sep) !== -1 ) { - // Try interpreting `str` as path to a file. - return fs.readFileSync(str, {encoding: 'utf8'}); + try { + // Try interpreting `str` as path to a file. + return fs.readFileSync(str, {encoding: 'utf8'}); + } catch (err) { + // If the file doesn't exist, return `null`. Rethrow all other errors. + if (err.code !== 'ENOENT') throw err; + } } return null; } From 7cdd7a6f32ba5b74fb4991ead410bfa14485db4e Mon Sep 17 00:00:00 2001 From: valadaptive Date: Thu, 22 Feb 2024 00:51:13 -0500 Subject: [PATCH 9/9] Remove unused files.js exports --- etc/browser/lib/files.js | 2 -- lib/files.js | 3 --- 2 files changed, 5 deletions(-) diff --git a/etc/browser/lib/files.js b/etc/browser/lib/files.js index b984631f..20d0934f 100644 --- a/etc/browser/lib/files.js +++ b/etc/browser/lib/files.js @@ -17,7 +17,5 @@ function tryReadFileSync() { return null; } module.exports = { createImportHook, createSyncImportHook, - existsSync: function () { return false; }, - readFileSync: function () { throw createError(); }, tryReadFileSync, }; diff --git a/lib/files.js b/lib/files.js index b9405f46..f46530fe 100644 --- a/lib/files.js +++ b/lib/files.js @@ -76,8 +76,5 @@ function tryReadFileSync(str) { module.exports = { createImportHook, createSyncImportHook, - // Proxy a few methods to better shim them for browserify. - existsSync: fs.existsSync, - readFileSync: fs.readFileSync, tryReadFileSync };