Fix too many bins protection for histogram distribution back end calls (#1745)

* get jest setup working again

* override transformIgnorePatterns to also ignore d3

* add test for too many bins failure and fix

* renamed and reused max bins constant

Author: bobular

packages/libs/eda/config-overrides.js

@@ -58,3 +58,25 @@ module.exports = function override(config, env) {
     },
   };
 };
+
+/*
+ * Jest (test runner) config overrides, applied by react-app-rewired.
+ *
+ * Several modules in this package transitively import the d3 family of
+ * packages (e.g. d3, d3-scale), which ship as untransformed ES modules.
+ * CRA's default `transformIgnorePatterns` skips everything in node_modules,
+ * so Jest chokes on the `export` syntax with "Unexpected token 'export'".
+ *
+ * react-app-rewired *concatenates* array overrides from package.json onto
+ * CRA's defaults, so we can't relax the pattern there - the broad default
+ * still matches. Instead we replace `transformIgnorePatterns` outright here,
+ * using a negative lookahead so the d3 packages (and their deps) ARE
+ * transformed while everything else in node_modules is still skipped.
+ */
+module.exports.jest = function (config) {
+  config.transformIgnorePatterns = [
+    '[/\\\\]node_modules[/\\\\](?!(?:d3|d3-[\\w-]+|internmap|delaunator|robust-predicates)[/\\\\]).+\\.(js|jsx|mjs|cjs|ts|tsx)$',
+    '^.+\\.module\\.(css|sass|scss)$',
+  ];
+  return config;
+};

packages/libs/eda/package.json

@@ -19,6 +19,7 @@
     "react-cool-dimensions": "^2.0.7",
     "react-draggable": "^4.4.3",
     "react-resizable": "^3.0.4",
+    "react-test-renderer": "^18.0.0",
     "uuid": "^8.3.2"
   },
   "peerDependencies": {
@@ -65,7 +66,7 @@
     "@tanstack/react-query-devtools": "^4.35.3",
     "@testing-library/jest-dom": "^5.16.5",
     "@testing-library/react": "^11.1.0",
-    "@testing-library/react-hooks": "^5.0.3",
+    "@testing-library/react-hooks": "^8.0.1",
     "@testing-library/user-event": "^12.1.10",
     "@types/date-arithmetic": "^4.1.1",
     "@types/debounce-promise": "^3.1.3",
@@ -79,6 +80,7 @@
     "@types/react-resizable": "^1.7.2",
     "@types/react-router-dom": "^5.3.3",
     "@types/react-table": "^7.7.12",
+    "@types/react-test-renderer": "^18",
     "@types/uuid": "^8.3.4",
     "@veupathdb/browserslist-config": "workspace:^",
     "@veupathdb/eslint-config": "workspace:^",
@@ -90,6 +92,7 @@
     "buffer": "^6.0.3",
     "http-proxy-middleware": "^2.0.6",
     "ify-loader": "^1.1.0",
+    "jest-watch-typeahead": "^3.0.1",
     "notistack": "^3.0.1",
     "path-browserify": "^1.0.1",
     "postcss-normalize": "^10.0.1",
@@ -109,5 +112,11 @@
     "stream-browserify": "^3.0.0",
     "typescript": "^4.3.4",
     "web-vitals": "^0.2.4"
+  },
+  "jest": {
+    "testPathIgnorePatterns": [
+      "/node_modules/",
+      "useAnalysis\\.test\\.tsx"
+    ]
   }
 }

packages/libs/eda/src/lib/core/constants.ts

@@ -0,0 +1,9 @@
+/**
+ * The maximum number of bins the backend will produce for a single
+ * `distribution` request. Exceeding it yields a bad-request response:
+ * "Variable ... Maximum number of allowed bins (2000) exceeded."
+ *
+ * Front-end code that computes histogram ranges/bin widths must keep
+ * `(rangeMax - rangeMin) / binWidth` at or below this limit.
+ */
+export const MAX_DISTRIBUTION_BINS = 2000;

packages/libs/eda/src/lib/core/utils/default-axis-range.test.ts

@@ -0,0 +1,72 @@
+import { numberDateDefaultAxisRange } from './default-axis-range';
+import { NumberVariable } from '../types/study';
+import { NumberRange } from '../types/general';
+import { MAX_DISTRIBUTION_BINS } from '../constants';
+
+function makeIntegerVariable(
+  distributionDefaults: NumberVariable['distributionDefaults']
+): NumberVariable {
+  return {
+    id: 'VAR_TEST',
+    providerLabel: 'test',
+    displayName: 'Test integer variable',
+    hideFrom: [],
+    type: 'integer',
+    units: '',
+    dataShape: 'continuous',
+    distinctValuesCount: 5,
+    isTemporal: false,
+    isFeatured: false,
+    isMergeKey: false,
+    isMultiValued: false,
+    distributionDefaults,
+  };
+}
+
+describe('numberDateDefaultAxisRange - max bins protection', () => {
+  // Real-life bug: a year-like integer variable with values 2000..2004,
+  // continuous, binWidth 1. Because rangeMin (2000) !== rangeMax (2004),
+  // the histogram's lower bound was set to 0, producing 2004 bins for a
+  // displayRangeMin..displayRangeMax of 0..2004, exceeding the backend's
+  // 2000-bin limit. https://github.com/VEuPathDB/web-monorepo (year integers)
+  it('does not produce more than the max allowed bins for a large-valued integer variable', () => {
+    const binWidth = 1;
+    const variable = makeIntegerVariable({
+      rangeMin: 2000,
+      rangeMax: 2004,
+      binWidth,
+    });
+
+    const range = numberDateDefaultAxisRange(
+      variable,
+      undefined,
+      undefined,
+      undefined
+    ) as NumberRange;
+
+    const binCount = (range.max - range.min) / binWidth;
+    expect(binCount).toBeLessThanOrEqual(MAX_DISTRIBUTION_BINS);
+
+    // and the range must still cover the data
+    expect(range.min).toBeLessThanOrEqual(2000);
+    expect(range.max).toBeGreaterThanOrEqual(2004);
+  });
+
+  it('still starts ordinary positive integer variables at zero', () => {
+    const variable = makeIntegerVariable({
+      rangeMin: 5,
+      rangeMax: 100,
+      binWidth: 1,
+    });
+
+    const range = numberDateDefaultAxisRange(
+      variable,
+      undefined,
+      undefined,
+      undefined
+    ) as NumberRange;
+
+    expect(range.min).toBe(0);
+    expect(range.max).toBe(100);
+  });
+});

packages/libs/eda/src/lib/core/utils/default-axis-range.ts

@@ -2,6 +2,7 @@ import { NumberDistributionDefaults, Variable } from '../types/study';
 import { NumberOrDateRange } from '@veupathdb/components/lib/types/general';
 // type of computedVariableMetadata for computation apps such as alphadiv and abundance
 import { VariableMapping } from '../api/DataClient/types';
+import { MAX_DISTRIBUTION_BINS } from '../constants';
 import { min, max } from 'lodash';
 
 export function numberDateDefaultAxisRange(
@@ -138,19 +139,19 @@ export function numberDateDefaultAxisRange(
   } else return undefined;
 }
 
-const MAX_EDGE_CASE_BINS = 2000;
-
 /**
  * The basic functionality is to return `min([ displayRangeMin ?? 0, rangeMin, observedMin ])`
  * The zero is targeted at histograms which should start at zero even if rangeMin > 0.
  *
- * However, there is an edge case when rangeMin === rangeMax. (When the default binWidth defaults to 1.)
+ * However, there is an edge case for variables whose values are clustered far from zero
+ * (e.g. a year-like integer with values 2000..2004 and a default `binWidth` of 1).
  * If we are making a `distribution` histogram request in this scenario, we do NOT want
- * to request `{ min: 0, max: rangeMax, binWidth: 1 }`
- * because this will cause an OOM error on the backend if `rangeMax` is large.
+ * to anchor the lower bound at zero and request `{ min: 0, max: rangeMax, binWidth: 1 }`
+ * because the bin count (`rangeMax / binWidth`) would exceed the backend's limit and
+ * produce a "Maximum number of allowed bins exceeded" error.
  *
- * The solution is to check for the edge case and return
- * `rangeMin - binWidth` for the lower bound if the number of bins would be excessive.
+ * The solution is to detect when anchoring at zero would create an excessive number of
+ * bins and instead use `rangeMin - binWidth` for the lower bound.
  */
 function getSafeLowerBound(
   { displayRangeMin, rangeMin, rangeMax, binWidth }: NumberDistributionDefaults,
@@ -162,9 +163,9 @@ function getSafeLowerBound(
   if (displayRangeMin != null) {
     lowerBound = displayRangeMin;
   } else if (
-    // check for the edge-case
-    rangeMin === rangeMax &&
-    rangeMax / (binWidth ?? 1) > MAX_EDGE_CASE_BINS
+    // would anchoring the lower bound at zero create too many bins?
+    rangeMax / (binWidth ?? 1) >
+    MAX_DISTRIBUTION_BINS
   ) {
     lowerBound = rangeMin - (binWidth ?? 1);
   }