Skip to content

optional determinisic sort order #465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: meteor-2.x
Choose a base branch
from
+107 −108
Open

optional determinisic sort order #465

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
68b3e7e
Ensure determinisic sort by appending _id: 1 always
lynchem Apr 8, 2025
4a927a8
make the deterministic sort optional
lynchem Apr 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .npm/package/npm-shrinkwrap.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 6 additions & 7 deletions client/main.js
View file
Original file line number Diff line number Diff line change
@@ -112,7 +112,8 @@ Template.tabular.onRendered(function () {
}

// Update sort
template.tabular.sort.set(getMongoSort(data.order, options.columns));
const deterministic = template.tabular.tableDef.ensureDeterministicSort ?? false;
template.tabular.sort.set(getMongoSort(data.order, options.columns, deterministic));

// Update pubSelector
let pubSelector = template.tabular.selector;
@@ -473,11 +474,10 @@ Template.tabular.onRendered(function () {

if (template.tabular.blazeViews) {
//console.log(`Removing ${template.blazeViews.length}`);
template.tabular.blazeViews.forEach(view => {
template.tabular.blazeViews.forEach((view) => {
try {
Blaze.remove(view);
}
catch(err) {
} catch (err) {
console.error(err);
}
});
@@ -529,11 +529,10 @@ Template.tabular.onDestroyed(function () {

if (this.tabular?.blazeViews) {
//console.log(`Removing ${this.blazeViews.length}`);
this.tabular.blazeViews.forEach(view => {
this.tabular.blazeViews.forEach((view) => {
try {
Blaze.remove(view);
}
catch(err) {
} catch (err) {
console.error(err);
}
});
2 changes: 2 additions & 0 deletions common/Tabular.js
View file
Original file line number Diff line number Diff line change
@@ -46,6 +46,7 @@ Tabular.Table = class {
this.skipCount = options.skipCount;
this.searchCustom = options.searchCustom;
this.searchExtraFields = options.searchExtraFields;
this.ensureDeterministicSort = options.ensureDeterministicSort;

if (_.isArray(options.extraFields)) {
const fields = {};
@@ -69,6 +70,7 @@ Tabular.Table = class {
'throttleRefresh',
'extraFields',
'searchExtraFields',
'ensureDeterministicSort',
'alternativeCount',
'skipCount',
'name',
11 changes: 10 additions & 1 deletion common/util.js
View file
Original file line number Diff line number Diff line change
@@ -48,7 +48,7 @@ export function objectsAreEqual(oldVal, newVal) {

// Take the DataTables `order` format and column info
// and convert it into a mongo sort array.
export function getMongoSort(order, columns) {
export function getMongoSort(order, columns, deterministic) {
if (!order || !columns) {
return;
}
@@ -71,6 +71,15 @@ export function getMongoSort(order, columns) {
sort.push([propName, dir]);
}
});

// Add _id as a tie-breaker to ensure deterministic sorting
if (deterministic && sort.length > 0) {
// Only add _id if it's not already in the sort array
if (!sort.some(([field]) => field === '_id')) {
sort.push(['_id', 'asc']);
}
}

return sort;
}

187 changes: 88 additions & 99 deletions tests/util.js
View file
Original file line number Diff line number Diff line change
@@ -1,117 +1,106 @@
// Test really reliable Util functions
Tinytest.add('Util - cleanFieldName', function (test) {
var Input = "Parents.Child[0]"
var ExpectedOutput = "Parents"
var Output = Util.cleanFieldName(Input)
LogResults(Input, ExpectedOutput, Output, test)
})
var Input = 'Parents.Child[0]';
var ExpectedOutput = 'Parents';
var Output = Util.cleanFieldName(Input);
LogResults(Input, ExpectedOutput, Output, test);
});
Tinytest.add('Util - cleanFieldNameForSearch', function (test) {
var Input = 'Parents.Child[0]'
var ExpectedOutput = "Parents.Child"
var Output = Util.cleanFieldNameForSearch(Input)
LogResults(Input, ExpectedOutput, Output, test)
})
var Input = 'Parents.Child[0]';
var ExpectedOutput = 'Parents.Child';
var Output = Util.cleanFieldNameForSearch(Input);
LogResults(Input, ExpectedOutput, Output, test);
});
Tinytest.add('Util - sortsAreEqual', function (test) {
var Input = ["Parents", "Child"]
var ExpectedOutput = false
var Output = Util.sortsAreEqual(Input[0], Input[1])
LogResults(Input, ExpectedOutput, Output, test)
})
var Input = ['Parents', 'Child'];
var ExpectedOutput = false;
var Output = Util.sortsAreEqual(Input[0], Input[1]);
LogResults(Input, ExpectedOutput, Output, test);
});
Tinytest.add('Util - objectsAreEqual', function (test) {
var Input = [{Child: 0}, {Child: 1}]
var ExpectedOutput = false
var Output = Util.objectsAreEqual(Input[0], Input[1])
LogResults(Input, ExpectedOutput, Output, test)
})

var Input = [{ Child: 0 }, { Child: 1 }];
var ExpectedOutput = false;
var Output = Util.objectsAreEqual(Input[0], Input[1]);
LogResults(Input, ExpectedOutput, Output, test);
});

//
// More complex Util Functions
//
Tinytest.add('Util - getMongoSort', function (test) {
// Note sort does not work on columns run through the parseMultiField
// function because the order of the columns in the array changes,
// instead, the first class in a spaced-separated list is used
var SpacedClassList = ["ClassOne", "ClassTwo ClassThree"]
var BothCols = GenerateBothColumns(SpacedClassList)
var order = [{
column: 1,
dir: 'asc'
}]
// var ExpectedOutput = [["ClassTwo","asc"]]
var ExpectedOutput = [["url","asc"]]
var Output = Util.getMongoSort(order, BothCols.columns)
LogResults(BothCols.columns, ExpectedOutput, Output, test)
})
// Note sort does not work on columns run through the parseMultiField
// function because the order of the columns in the array changes,
// instead, the first class in a spaced-separated list is used
var SpacedClassList = ['ClassOne', 'ClassTwo ClassThree'];
var BothCols = GenerateBothColumns(SpacedClassList);
var order = [
{
column: 1,
dir: 'asc'
}
];
// var ExpectedOutput = [["ClassTwo","asc"]]
var ExpectedOutput = [['url', 'asc']];
var Output = Util.getMongoSort(order, BothCols.columns, false);
LogResults(BothCols.columns, ExpectedOutput, Output, test);
});

Tinytest.add('Util - parseMultiFieldColumns', function (test) {
var SpacedClassList = ["one two", Fake.sentence([4]), Fake.sentence([5])]
var BothCols = GenerateBothColumns(SpacedClassList)
var Output = Util.parseMultiFieldColumns(BothCols.columns)
LogResults(BothCols.columns, BothCols.ExpectedOutput, Output, test)
})
var SpacedClassList = ['one two', Fake.sentence([4]), Fake.sentence([5])];
var BothCols = GenerateBothColumns(SpacedClassList);
var Output = Util.parseMultiFieldColumns(BothCols.columns);
LogResults(BothCols.columns, BothCols.ExpectedOutput, Output, test);
});

Tinytest.add('Util - createRegExp', function (test) {
//
// Basic Use Case
//
var SpacedClassList = ["ClassOne"]
var searchString = 'TestSearch'
// This must be an object and not an array:
var Input = createRegExpField(SpacedClassList, searchString, {})[0]
//
// Basic Use Case
//
var SpacedClassList = ['ClassOne'];
var searchString = 'TestSearch';
// This must be an object and not an array:
var Input = createRegExpField(SpacedClassList, searchString, {})[0];

var ExpectedOutput = searchString
var Output = Util.createRegExp(Input, searchString)
LogResults(Input, ExpectedOutput, Output, test)
var ExpectedOutput = searchString;
var Output = Util.createRegExp(Input, searchString);
LogResults(Input, ExpectedOutput, Output, test);

//
// Now with a RegExp
//
var PassedOptions = {
regex: ['^\\D', '\\D?', '*']
// regex: '^\\D'
}
var Input = createRegExpField(
SpacedClassList,
searchString,
PassedOptions
)[0]
var ElasticSearchString = searchString.replace(/(.)/g, '$1'+
PassedOptions.regex[1]);
var ExpectedOutput = PassedOptions.regex[0]+
ElasticSearchString+PassedOptions.regex[2]
// var ExpectedOutput = PassedOptions.regex+searchString
// This must be an object and not an array:
var Output = Util.createRegExp(Input, searchString)
LogResults(Input, ExpectedOutput, Output, test)
//
// Now with a RegExp
//
var PassedOptions = {
regex: ['^\\D', '\\D?', '*']
// regex: '^\\D'
};
var Input = createRegExpField(SpacedClassList, searchString, PassedOptions)[0];
var ElasticSearchString = searchString.replace(/(.)/g, '$1' + PassedOptions.regex[1]);
var ExpectedOutput = PassedOptions.regex[0] + ElasticSearchString + PassedOptions.regex[2];
// var ExpectedOutput = PassedOptions.regex+searchString
// This must be an object and not an array:
var Output = Util.createRegExp(Input, searchString);
LogResults(Input, ExpectedOutput, Output, test);

//
// With the proposed "limit" term
// Where only the first two letters are searched
//
var PassedOptions = {
regex: ['^\\D', '\\D?', '*', 2]
}
var Input = createRegExpField(
SpacedClassList,
searchString.match(/\D{2}/),
PassedOptions
)[0]
// Take only first two letters
searchString = searchString[0]+searchString[1]
var ElasticSearchString = searchString.replace(
/(.)/g, '$1'+ PassedOptions.regex[1]);
var ExpectedOutput = PassedOptions.regex[0]+
ElasticSearchString+PassedOptions.regex[2]
// This must be an object and not an array:
var Output = Util.createRegExp(Input, searchString)
LogResults(Input, ExpectedOutput, Output, test)
//
// With the proposed "limit" term
// Where only the first two letters are searched
//
var PassedOptions = {
regex: ['^\\D', '\\D?', '*', 2]
};
var Input = createRegExpField(SpacedClassList, searchString.match(/\D{2}/), PassedOptions)[0];
// Take only first two letters
searchString = searchString[0] + searchString[1];
var ElasticSearchString = searchString.replace(/(.)/g, '$1' + PassedOptions.regex[1]);
var ExpectedOutput = PassedOptions.regex[0] + ElasticSearchString + PassedOptions.regex[2];
// This must be an object and not an array:
var Output = Util.createRegExp(Input, searchString);
LogResults(Input, ExpectedOutput, Output, test);

//
// Where a third letter is searched (i.e. not searched):
//
searchString = searchString+'3'
var ExpectedOutput = '^@&&@&&@&&@&&@&&@&&@'
var Output = Util.createRegExp(Input, searchString)
LogResults(Input, ExpectedOutput, Output, test)
})
//
// Where a third letter is searched (i.e. not searched):
//
searchString = searchString + '3';
var ExpectedOutput = '^@&&@&&@&&@&&@&&@&&@';
var Output = Util.createRegExp(Input, searchString);
LogResults(Input, ExpectedOutput, Output, test);
});