HAR-10247: add validator rules to clean up numberingxml (#800)

* feat: adding xml validator with unittests: numbering

Author: falsaadehh

packages/super-editor/src/core/Editor.js

@@ -1541,6 +1541,8 @@ export class Editor extends EventEmitter {
       fieldsHighlightColor,
     );
 
+    this.#validateDocumentExport();
+
     if (exportXmlOnly || exportJsonOnly) return documentXml;
 
     const customXml = this.converter.schemaToXml(this.converter.convertedXml['docProps/custom.xml'].elements[0]);
@@ -1888,4 +1890,16 @@ export class Editor extends EventEmitter {
     const validator = new SuperValidator({ editor: this, dryRun: false, debug: false });
     validator.validateActiveDocument();
   }
+
+  /**
+   * Run the SuperValidator's on document upon export to check and fix potential known issues.
+   * @returns {void}
+   */
+  #validateDocumentExport() {
+    if (this.options.isHeaderOrFooter || this.options.isChildEditor) return;
+
+    /** @type {import('./super-validator/index.js').SuperValidator} */
+    const validator = new SuperValidator({ editor: this, dryRun: false, debug: false });
+    validator.validateDocumentExport();
+  }
 }

packages/super-editor/src/core/super-converter/v2/importer/docxImporter.js

@@ -547,7 +547,9 @@ function getNumberingDefinitions(docx) {
   let importListDefs = {};
   definitions.forEach((el) => {
     const numId = Number(el.attributes['w:numId']);
-    importListDefs[numId] = el;
+    if (Number.isInteger(numId)) {
+      importListDefs[numId] = el;
+    }
   });
 
   const listDefsEntries = Object.entries(importListDefs);

packages/super-editor/src/core/super-validator/super-validator.js

@@ -1,6 +1,7 @@
 // @ts-check
 import { createLogger } from './logger/logger.js';
 import { StateValidators } from './validators/state/index.js';
+import { XmlValidators } from './validators/xml/index.js';
 
 /**
  * @typedef {import('./types.js').ElementInfo} ElementInfo
@@ -25,6 +26,9 @@ export class SuperValidator {
   /** @type {any} */
   #stateValidators;
 
+  /** @type {any} */
+  #xmlValidators;
+
   /** @type {Set<string>} */
   #requiredNodeTypes;
 
@@ -42,35 +46,42 @@ export class SuperValidator {
     this.logger = createLogger(this.debug);
 
     // Initialize validators and collect their requirements
-    const { validators, nodeTypes, markTypes } = this.#initializeValidators();
-    this.#stateValidators = validators;
+    const { stateValidators, xmlValidators, nodeTypes, markTypes } = this.#initializeValidators();
+    this.#stateValidators = stateValidators;
+    this.#xmlValidators = xmlValidators;
     this.#requiredNodeTypes = nodeTypes;
     this.#requiredMarkTypes = markTypes;
   }
 
   /**
    * Initialize all validators and collect their element requirements
-   * @returns {{ validators: Record<string, ValidatorFunction>, nodeTypes: Set<string>, markTypes: Set<string> }}
+   * @returns {{ stateValidators: Record<string, ValidatorFunction>, xmlValidators: Record<string, ValidatorFunction>, nodeTypes: Set<string>, markTypes: Set<string> }}
    */
   #initializeValidators() {
     const requiredNodes = new Set();
     const requiredMarks = new Set();
 
-    const validators = Object.fromEntries(
-      Object.entries(StateValidators).map(([key, factory]) => {
-        const validatorLogger = this.logger.withPrefix(key);
-        /** @type {ValidatorFunction} */
-        const validator = factory({ editor: this.#editor, logger: validatorLogger });
+    const initializeValidatorSet = (validatorFactories) => {
+      return Object.fromEntries(
+        Object.entries(validatorFactories).map(([key, factory]) => {
+          const validatorLogger = this.logger.withPrefix(key);
+          /** @type {ValidatorFunction} */
+          const validator = factory({ editor: this.#editor, logger: validatorLogger });
+
+          // Collect requirements from this validator
+          this.#collectValidatorRequirements(validator, requiredNodes, requiredMarks);
 
-        // Collect requirements from this validator
-        this.#collectValidatorRequirements(validator, requiredNodes, requiredMarks);
+          return [key, validator];
+        }),
+      );
+    };
 
-        return [key, validator];
-      }),
-    );
+    const stateValidators = initializeValidatorSet(StateValidators);
+    const xmlValidators = initializeValidatorSet(XmlValidators);
 
     return {
-      validators,
+      stateValidators: stateValidators,
+      xmlValidators: xmlValidators,
       nodeTypes: requiredNodes,
       markTypes: requiredMarks,
     };
@@ -174,4 +185,32 @@ export class SuperValidator {
     this.logger.debug('Results:', validationResults);
     return { modified: hasModifiedDocument, results: validationResults };
   }
+
+  /**
+   * Validate the exported document in the editor. Triggered automatically on editor export.
+   * @returns {{ modified: boolean, results: Array<{ key: string, results: string[] }> }}
+   */
+  validateDocumentExport() {
+    const { tr } = this.#editor.state;
+    const { dispatch } = this.#editor.view;
+
+    let hasModifiedDocument = false;
+    const validationResults = [];
+
+    // Run XML validators
+    Object.entries(this.#xmlValidators).forEach(([key, validator]) => {
+      this.logger.debug(`🕵 Validating export with ${key}...`);
+
+      const { results, modified } = validator();
+      validationResults.push({ key, results });
+
+      hasModifiedDocument = hasModifiedDocument || modified;
+    });
+
+    if (!this.dryRun && hasModifiedDocument) dispatch(tr);
+    else this.logger.debug('DRY RUN: No export changes applied to the document.');
+
+    this.logger.debug('Export validation results:', validationResults);
+    return { modified: hasModifiedDocument, results: validationResults };
+  }
 }

packages/super-editor/src/core/super-validator/super-validator.test.js

@@ -1,6 +1,7 @@
-import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
 import { SuperValidator } from './super-validator.js';
 import { StateValidators } from './validators/state/index.js';
+import { XmlValidators } from './validators/xml/index.js';
 
 vi.mock('./logger/logger.js', () => ({
   createLogger: vi.fn(() => ({
@@ -18,6 +19,17 @@ vi.mock('./validators/state/index.js', () => ({
   },
 }));
 
+vi.mock('./validators/xml/index.js', () => {
+  // Provide safe default factories for tests
+  // that doesn't have export and doesn't need mock xml validators
+  return {
+    XmlValidators: {
+      xmlA: vi.fn(() => ({ modified: false, results: [] })),
+      xmlB: vi.fn(() => ({ modified: false, results: [] })),
+    },
+  };
+});
+
 describe('SuperValidator', () => {
   let mockEditor, mockDoc, mockView, mockTr;
 
@@ -114,4 +126,54 @@ describe('SuperValidator', () => {
 
     expect(result.results).toHaveLength(2);
   });
+
+  describe('validateDocumentExport', () => {
+    it('calls all XML validators and aggregates results; dispatches when modified', () => {
+      const xmlValidatorA = vi.fn(() => ({ modified: true, results: ['fixed numbering'] }));
+      const xmlValidatorB = vi.fn(() => ({ modified: false, results: [] }));
+
+      XmlValidators.xmlA.mockReturnValue(xmlValidatorA);
+      XmlValidators.xmlB.mockReturnValue(xmlValidatorB);
+
+      const instance = new SuperValidator({ editor: mockEditor });
+
+      const result = instance.validateDocumentExport();
+
+      expect(result.modified).toBe(true);
+      expect(result.results).toEqual([
+        { key: 'xmlA', results: ['fixed numbering'] },
+        { key: 'xmlB', results: [] },
+      ]);
+
+      expect(mockView.dispatch).toHaveBeenCalled();
+    });
+
+    it('does not dispatch if no XML validator modified the document', () => {
+      const xmlValidatorA = vi.fn(() => ({ modified: false, results: [] }));
+      const xmlValidatorB = vi.fn(() => ({ modified: false, results: [] }));
+
+      XmlValidators.xmlA.mockReturnValue(xmlValidatorA);
+      XmlValidators.xmlB.mockReturnValue(xmlValidatorB);
+
+      const instance = new SuperValidator({ editor: mockEditor });
+      const result = instance.validateDocumentExport();
+
+      expect(result.modified).toBe(false);
+      expect(mockView.dispatch).not.toHaveBeenCalled();
+    });
+
+    it('does not dispatch if dryRun is true even when modified', () => {
+      const xmlValidatorA = vi.fn(() => ({ modified: true, results: ['something'] }));
+      const xmlValidatorB = vi.fn(() => ({ modified: false, results: [] }));
+
+      XmlValidators.xmlA.mockReturnValue(xmlValidatorA);
+      XmlValidators.xmlB.mockReturnValue(xmlValidatorB);
+
+      const instance = new SuperValidator({ editor: mockEditor, dryRun: true });
+      const result = instance.validateDocumentExport();
+
+      expect(result.modified).toBe(true);
+      expect(mockView.dispatch).not.toHaveBeenCalled();
+    });
+  });
 });

packages/super-editor/src/core/super-validator/types.js

@@ -20,6 +20,13 @@
  * @returns {{ modified: boolean, results: string[] }} - Validation results and whether the document was modified.
  */
 
+/**
+ * @typedef {function} XmlValidator
+ * @param {Editor} editor - The editor instance to validate.
+ * @param {ValidatorLogger} logger - Logger for validation messages.
+ * @returns {{ modified: boolean, results: string[] }} - Validation results and whether the document was modified.
+ */
+
 /**
  * @typedef {Object} ElementInfo
  * @property {import('prosemirror-model').Node} [node]

packages/super-editor/src/core/super-validator/validators/xml/index.js

@@ -0,0 +1,10 @@
+// @ts-check
+import { createNumberingValidator } from './numbering/numbering-validator.js';
+
+/**
+ * @typedef {Object} XmlValidator
+ * @property {import('../../types.js').XmlValidator} numberingValidator - Validator for numbering.xml file.
+ */
+export const XmlValidators = {
+  numberingValidator: createNumberingValidator,
+};

packages/super-editor/src/core/super-validator/validators/xml/numbering/numbering-validator.js

@@ -0,0 +1,65 @@
+// @ts-check
+/**
+ * @typedef {import('../../../types.js').Editor} Editor
+ * @typedef {import('../../../types.js').ValidatorLogger} ValidatorLogger
+ */
+export function createNumberingValidator({ editor, logger }) {
+  return () => {
+    const results = [];
+    let modified = false;
+
+    const convertedXml = editor?.converter?.convertedXml;
+    const path = 'word/numbering.xml';
+    const numbering = convertedXml?.[path];
+
+    if (!numbering || !numbering.elements?.length || !numbering.elements[0].elements?.length) {
+      results.push(`${path} is not a valid xml`);
+      return { results, modified };
+    }
+
+    const removed = [];
+    let elements = numbering.elements[0];
+    pruneInvalidNumNodes(elements, removed);
+
+    if (removed.length) {
+      modified = true;
+      results.push(`Removed invalid <w:num> by numId:` + removed.join(', '));
+      logger?.debug?.(`Removed invalid <w:num> by numId: ${removed.join(', ')}`);
+    } else {
+      results.push('No <w:num> entries with null/invalid numId found.');
+    }
+    return { results, modified };
+  };
+}
+/**
+ * Recursively walks an xml tree and removes <w:num> with bad numId.
+ * Mutates the tree in place.
+ * @param {any} node - xml node (expects .elements array for element nodes)
+ * @param {string[]} removed - collects removed numId values
+ */
+function pruneInvalidNumNodes(node, removed) {
+  if (!node || !Array.isArray(node.elements)) return;
+
+  const next = [];
+  for (const el of node.elements) {
+    if (el?.type === 'element') {
+      if (el.name === 'w:num') {
+        const attrs = el.attributes || {};
+        const raw = attrs['w:numId'];
+        const v = raw == null ? null : String(raw).trim();
+        const isInvalid = v == null || v === '' || /^null$/i.test(v) || !/^\d+$/.test(v);
+
+        if (isInvalid) {
+          removed.push(v ?? 'missing node');
+          continue;
+        }
+      }
+      if (Array.isArray(el.elements) && el.elements.length) {
+        pruneInvalidNumNodes(el, removed);
+      }
+    }
+    next.push(el);
+  }
+
+  node.elements = next;
+}