Skip to content

Commit

Permalink
address various PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
brontolosone committed Dec 18, 2024
1 parent ea5c499 commit cb6a021
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 25 deletions.
13 changes: 7 additions & 6 deletions packages/xpath/src/functions/xforms/node-set.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import sha256 from 'crypto-js/sha256';
import { SHA256 } from 'crypto-js';

import type { XPathNode } from '../../adapter/interface/XPathNode.ts';
import type { XPathDOMProvider } from '../../adapter/xpathDOMProvider.ts';
Expand Down Expand Up @@ -410,17 +410,18 @@ export const randomize = new NodeSetFunction(
);

const toBigIntHash = (text: string): bigint => {
// hash text with sha256, and interpret the first 64 bits of output
// Hash text with sha256, and interpret the first 64 bits of output
// (the first and second int32s ("words") of CryptoJS digest output)
// as a BigInt. Thus the entropy of the hash is reduced to 64 bits, which
// as an int64 (in JS represented in a BigInt).
// Thus the entropy of the hash is reduced to 64 bits, which
// for some applications is sufficient.
// The underlying representations are big-endian regardless of the endianness
// of the machine this runs on, as is the equivalent JavaRosa implementation
// at https://github.com/getodk/javarosa/blob/ab0e8f4da6ad8180ac7ede5bc939f3f261c16edf/src/main/java/org/javarosa/xpath/expr/XPathFuncExpr.java#L718-L726
const buffer = new ArrayBuffer(8);
const dataview = new DataView(buffer);
sha256(text)
SHA256(text)
.words.slice(0, 2)
.forEach((val, ix) => dataview.setInt32(ix * 4, val));
.forEach((val, ix) => dataview.setInt32(ix * Int32Array.BYTES_PER_ELEMENT, val));
return dataview.getBigInt64(0);
}
};
41 changes: 23 additions & 18 deletions packages/xpath/src/lib/collections/sort.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ class UnseededPseudoRandomNumberGenerator implements PseudoRandomNumberGenerator
}
}

class SeededPseudoRandomNumberGenerator implements PseudoRandomNumberGenerator {
// Park-Miller PRNG
class ParkMillerPRNG implements PseudoRandomNumberGenerator {
protected seed: number;

constructor(seed: Int | bigint) {
Expand Down Expand Up @@ -45,32 +44,38 @@ class SeededPseudoRandomNumberGenerator implements PseudoRandomNumberGenerator {
}
}

export const seededRandomize = <T>(values: readonly T[], seed?: number | bigint): T[] => {
let generator: PseudoRandomNumberGenerator;
class JavaRosaPRNG extends ParkMillerPRNG {
// Per issue #49 (https://github.com/getodk/web-forms/issues/49) this is intended to be "bug-or-feature-compatible"
// with JavaRosa's implementation; org.javarosa.core.model.ItemsetBinding.resolveRandomSeed takes the .longValue() of
// the double produced by randomSeedPathExpr.eval() — see https://github.com/getodk/javarosa/blob/6ce13527c/src/main/java/org/javarosa/core/model/ItemsetBinding.java#L311:L317 .
// That results in a 0L when the double is NaN, which happens (for instance) when there
// is a string that does not look like a number (which is a problem in itself, as any non-numeric
// looking string will then result in the same seed of 0 — see https://github.com/getodk/javarosa/issues/800).
// We'll emulate Java's Double -> Long conversion here (for NaN and some other double values)
// so that we produce the same randomization as JR.

if (seed == null) {
generator = new UnseededPseudoRandomNumberGenerator();
} else {
constructor(seed: Int | bigint) {
let finalSeed: number | bigint;
// Per issue #49 (https://github.com/getodk/web-forms/issues/49) this is intended to be "bug-or-feature-compatible"
// with JavaRosa's implementation; org.javarosa.core.model.ItemsetBinding.resolveRandomSeed takes the .longValue() of
// the double produced by randomSeedPathExpr.eval() — see https://github.com/getodk/javarosa/blob/6ce13527c/src/main/java/org/javarosa/core/model/ItemsetBinding.java#L311:L317 .
// That results in a 0L when the double is NaN, which happens (for instance) when there
// is a string that does not look like a number (which is a problem in itself, as any non-numeric
// looking string will then result in the same seed of 0 — see https://github.com/getodk/javarosa/issues/800).
// We'll emulate Java's Double -> Long conversion here (for NaN and some other double values)
// so that we produce the same randomization as JR.

// In Java, a NaN double's .longValue is 0
if (Number.isNaN(seed)) finalSeed = 0;
// In Java, an Infinity double's .longValue() is 2**63 -1, which is larger than Number.MAX_SAFE_INTEGER, thus we'll need a BigInt.
else if (seed === Infinity) finalSeed = 2n ** 63n - 1n;
// Analogous with the above conversion, but for -Infinity
else if (seed === -Infinity) finalSeed = -(2n ** 63n);
// A Java double's .longValue drops the fractional part.
// A Java Double's .longValue drops the fractional part.
else if (typeof seed === 'number' && !Number.isInteger(seed)) finalSeed = Math.trunc(seed);
else finalSeed = seed;
generator = new SeededPseudoRandomNumberGenerator(finalSeed);
super(finalSeed);
}
}

export const seededRandomize = <T>(values: readonly T[], seed?: number | bigint): T[] => {
let generator: PseudoRandomNumberGenerator;

if (seed == null) {
generator = new UnseededPseudoRandomNumberGenerator();
} else {
generator = new JavaRosaPRNG(seed);
}

const { length } = values;
Expand Down
2 changes: 1 addition & 1 deletion packages/xpath/test/xforms/randomize.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('randomize()', () => {

const SELECTOR = '//xhtml:div[@id="FunctionRandomize"]/xhtml:div';
const MIRROR = 'mirror';
const MIRROR_HASH_VALUE = 5989458117437254; // in Python: "from struct import unpack; from hashlib import sha256; unpack('>Q', sha256(b'mirror').digest()[:8])[0]"
const MIRROR_HASH_VALUE = 5989458117437254;
const MIRROR_HASH_SORT_ORDER = 'ACBEDF';

describe('shuffles nodesets', () => {
Expand Down

0 comments on commit cb6a021

Please sign in to comment.