« get me outta code hell

validators: optimize validateWikiData, support no-referenceType - hsmusic-wiki - HSMusic - static wiki software cataloguing collaborative creation
about summary refs log tree commit diff
diff options
context:
space:
mode:
author(quasar) nebula <qznebula@protonmail.com>2024-03-04 18:10:10 -0400
committer(quasar) nebula <qznebula@protonmail.com>2024-05-01 07:06:05 -0300
commited480d6b4dc489fa26f673d820f7f8bdd999c828 (patch)
treebf4449e288f59fba32dac3ba126a16dc5ce74061
parent262673f149f2b4ae8bc87d0e4fc507b7335609f4 (diff)
validators: optimize validateWikiData, support no-referenceType
-rw-r--r--src/data/thing.js16
-rw-r--r--src/data/validators.js43
-rw-r--r--test/unit/data/things/track.js1
3 files changed, 42 insertions, 18 deletions
diff --git a/src/data/thing.js b/src/data/thing.js
index 706e893d..9a8cec91 100644
--- a/src/data/thing.js
+++ b/src/data/thing.js
@@ -17,6 +17,22 @@ export default class Thing extends CacheableObject {
   static yamlDocumentSpec = Symbol.for('Thing.yamlDocumentSpec');
   static getYamlLoadingSpec = Symbol.for('Thing.getYamlLoadingSpec');
 
+  static isThingConstructor = Symbol.for('Thing.isThingConstructor');
+  static isThing = Symbol.for('Thing.isThing');
+
+  // To detect:
+  // Symbol.for('Thing.isThingConstructor') in constructor
+  static [Symbol.for('Thing.isThingConstructor')] = NaN;
+
+  static [CacheableObject.propertyDescriptors] = {
+    // To detect:
+    // Object.hasOwn(object, Symbol.for('Thing.isThing'))
+    [Symbol.for('Thing.isThing')]: {
+      flags: {expose: true},
+      expose: {compute: () => NaN},
+    },
+  };
+
   // Default custom inspect function, which may be overridden by Thing
   // subclasses. This will be used when displaying aggregate errors and other
   // command-line logging - it's the place to provide information useful in
diff --git a/src/data/validators.js b/src/data/validators.js
index c0cec8a9..5bc88210 100644
--- a/src/data/validators.js
+++ b/src/data/validators.js
@@ -606,8 +606,15 @@ export function isContentString(content) {
 export function isThingClass(thingClass) {
   isFunction(thingClass);
 
-  if (!Object.hasOwn(thingClass, Symbol.for('Thing.referenceType'))) {
-    throw new TypeError(`Expected a Thing constructor, missing Thing.referenceType`);
+  // This is *expressly* no faster than an instanceof check, because it's
+  // deliberately still walking the prototype chain for the provided object.
+  // (This is necessary because the symbol we're checking is defined only on
+  // the Thing constructor, and not directly on each subclass.) However, it's
+  // preferred over an instanceof check anyway, because instanceof would
+  // require that the #validators module has access to #thing, which it
+  // currently doesn't!
+  if (!(Symbol.for('Thing.isThingConstructor') in thingClass)) {
+    throw new TypeError(`Expected a Thing constructor, missing Thing.isThingConstructor`);
   }
 
   return true;
@@ -615,10 +622,13 @@ export function isThingClass(thingClass) {
 
 export function isThing(thing) {
   isObject(thing);
-  isFunction(thing.constructor);
 
-  if (!Object.hasOwn(thing.constructor, Symbol.for('Thing.referenceType'))) {
-    throw new TypeError(`Expected a Thing, constructor missing Thing.referenceType`);
+  // This *is* faster than an instanceof check, because it doesn't walk the
+  // prototype chain. It works because this property is set as part of every
+  // Thing subclass's inherited "public class fields" - it's set directly on
+  // every constructed Thing.
+  if (!Object.hasOwn(thing, Symbol.for('Thing.isThing'))) {
+    throw new TypeError(`Expected a Thing, missing Thing.isThing`);
   }
 
   return true;
@@ -798,25 +808,22 @@ export function validateWikiData({
       let foundOtherObject = false;
 
       for (const object of array) {
-        const {[Symbol.for('Thing.referenceType')]: referenceType} = object.constructor;
-
-        if (referenceType === undefined) {
-          foundOtherObject = true;
-
-          // Early-exit if a Thing has been found - nothing more can be learned.
-          if (foundThing) {
-            throw new TypeError(`Expected array of wiki data objects, got mixed items`);
-          }
-        } else {
-          foundThing = true;
-
+        if (Object.hasOwn(object, Symbol.for('Thing.isThing'))) {
           // Early-exit if a non-Thing object has been found - nothing more can
           // be learned.
           if (foundOtherObject) {
             throw new TypeError(`Expected array of wiki data objects, got mixed items`);
           }
 
-          allRefTypes.add(referenceType);
+          foundThing = true;
+          allRefTypes.add(object.constructor[Symbol.for('Thing.referenceType')]);
+        } else {
+          // Early-exit if a Thing has been found - nothing more can be learned.
+          if (foundThing) {
+            throw new TypeError(`Expected array of wiki data objects, got mixed items`);
+          }
+
+          foundOtherObject = true;
         }
       }
 
diff --git a/test/unit/data/things/track.js b/test/unit/data/things/track.js
index 57a297db..e8e2c7e6 100644
--- a/test/unit/data/things/track.js
+++ b/test/unit/data/things/track.js
@@ -248,6 +248,7 @@ t.test(`Track.color`, t => {
   track.albumData = [
     {
       constructor: {[Thing.referenceType]: 'album'},
+      [Thing.isThing]: true,
       color: '#abcdef',
       tracks: [track],
       trackSections: [