Skip to content

Commit

Permalink
fix: fixed infinite loop in expandUniqueDateBlocks()
Browse files Browse the repository at this point in the history
- identified and fixed scenario where expandUniqueDateBlocks() could loop infinitely
  • Loading branch information
dereekb committed Aug 11, 2022
1 parent 67f2dba commit 7464f2d
Show file tree
Hide file tree
Showing 2 changed files with 154 additions and 10 deletions.
116 changes: 114 additions & 2 deletions packages/date/src/lib/date/date.block.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { expectFail, itShouldFail } from '@dereekb/util/test';
import { DateRange, DateRangeInput } from './date.range';
import { addDays, addHours, addMinutes, setHours, setMinutes, startOfDay, endOfDay, addSeconds, addMilliseconds, millisecondsToHours, minutesToHours } from 'date-fns';
import { DateBlock, dateBlockDayOfWeekFactory, DateBlockIndex, dateBlockIndexRange, dateBlockRange, DateBlockRangeWithRange, dateBlocksExpansionFactory, dateBlocksInDateBlockRange, dateBlockTiming, DateBlockTiming, expandDateBlockRange, expandUniqueDateBlocks, getCurrentDateBlockTimingOffset, getCurrentDateBlockTimingStartDate, groupToDateBlockRanges, groupUniqueDateBlocks, isValidDateBlockTiming, UniqueDateBlockRange } from './date.block';
import { MS_IN_DAY, MINUTES_IN_DAY, range, RangeInput, Hours, Day } from '@dereekb/util';
import { DateBlock, dateBlockDayOfWeekFactory, DateBlockIndex, dateBlockIndexRange, dateBlockRange, DateBlockRangeWithRange, dateBlocksExpansionFactory, dateBlocksInDateBlockRange, dateBlockTiming, DateBlockTiming, expandDateBlockRange, expandUniqueDateBlocks, getCurrentDateBlockTimingOffset, getCurrentDateBlockTimingStartDate, groupToDateBlockRanges, groupUniqueDateBlocks, isValidDateBlockTiming, sortDateBlockRanges, UniqueDateBlockRange } from './date.block';
import { MS_IN_DAY, MINUTES_IN_DAY, range, RangeInput, Hours, Day, expandIndexSet } from '@dereekb/util';
import { removeMinutesAndSeconds } from './date';

describe('getCurrentDateBlockTimingOffset()', () => {
Expand Down Expand Up @@ -685,6 +685,23 @@ describe('expandUniqueDateBlocks', () => {
});
});
});

describe('with larger first followed by smaller', () => {
it('incumbent value should be retained.', () => {
const blocks = [
{ i: 0, to: 5, value: 'a' },
{ i: 0, value: 'b' }
];
const result = overwriteNextExpand(blocks);

expect(result.blocks.length).toBe(1);
expect(result.discarded.length).toBe(1);
expect(result.discarded[0].value).toBe(blocks[1].value);
expect(result.blocks[0].value).toBe(blocks[0].value);
expect(result.blocks[0].i).toBe(blocks[0].i);
expect(result.blocks[0].to).toBe(blocks[0].to);
});
});
});

describe('current', () => {
Expand Down Expand Up @@ -777,6 +794,25 @@ describe('expandUniqueDateBlocks', () => {
});
});
});

describe('with larger first followed by smaller', () => {
it('should start with the new value and end with the former value.', () => {
const blocks = [
{ i: 0, to: 5, value: 'a' },
{ i: 0, value: 'b' }
];
const result = overwriteNextExpand(blocks);

expect(result.blocks.length).toBe(2);
expect(result.discarded.length).toBe(0);
expect(result.blocks[0].value).toBe(blocks[1].value);
expect(result.blocks[1].value).toBe(blocks[0].value);
expect(result.blocks[0].i).toBe(blocks[1].i);
expect(result.blocks[0].to).toBe(blocks[1].to ?? 0);
expect(result.blocks[1].i).toBe(blocks[1].i + 1);
expect(result.blocks[1].to).toBe(blocks[0].to);
});
});
});
});

Expand Down Expand Up @@ -968,5 +1004,81 @@ describe('expandUniqueDateBlocks', () => {
});
});
});

describe('scenarios', () => {
it('should expand the blocks and not infinitely loop if another block comes after that has a 0-0 range.', async () => {
const i = 0;
const to = 2;

const allBlocks: any[] = [
{
i: 0,
to: 2
},
{ i: 0, to: undefined }
];

const requestedRangeBlocks = expandUniqueDateBlocks({
startAtIndex: i,
endAtIndex: to,
fillOption: 'fill',
overwriteOption: 'current',
fillFactory: (x) => x
})(allBlocks);

expect(requestedRangeBlocks.blocks).toBeDefined();
expect(requestedRangeBlocks.blocks.length).toBe(2);
expect(requestedRangeBlocks.blocks[0].i).toBe(0);
expect(requestedRangeBlocks.blocks[0].to).toBe(0);
expect(requestedRangeBlocks.blocks[1].i).toBe(1);
expect(requestedRangeBlocks.blocks[1].to).toBe(2);
});
});
});
});

describe('sortDateBlockRanges', () => {
it('should retain the order for items that have the same start index.', () => {
const allBlocks = [
{
i: 0,
to: 2,
id: 'a'
},
{ i: 0, to: undefined, id: 'b' },
{ i: 0, to: 3, id: 'c' },
{ i: 0, to: 1, id: 'd' },
{ i: 0, to: 100, id: 'e' }
];

const sorted = sortDateBlockRanges(allBlocks);

expect(sorted[0].id).toBe('a');
expect(sorted[1].id).toBe('b');
expect(sorted[2].id).toBe('c');
expect(sorted[3].id).toBe('d');
expect(sorted[4].id).toBe('e');
});

it('should retain the before/after order for items that have the same start index.', () => {
const allBlocks = [
{
i: 0,
to: 2,
id: 'a'
},
{ i: 1, to: undefined, id: 'b' },
{ i: 2, to: 3, id: 'c' },
{ i: 3, to: 1, id: 'd' },
{ i: 0, to: 100, id: 'e' }
];

const sorted = sortDateBlockRanges(allBlocks);

expect(sorted[0].id).toBe('a');
expect(sorted[1].id).toBe('e');
expect(sorted[2].id).toBe('b');
expect(sorted[3].id).toBe('c');
expect(sorted[4].id).toBe('d');
});
});
48 changes: 40 additions & 8 deletions packages/date/src/lib/date/date.block.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { DayOfWeek, RequiredOnKeys, IndexNumber, IndexRange, indexRangeCheckFunction, IndexRef, MINUTES_IN_DAY, MS_IN_DAY, sortAscendingIndexNumberRefFunction, UniqueModel, lastValue, FactoryWithRequiredInput, FilterFunction, mergeFilterFunctions, range, Milliseconds, Hours, MapFunction, getNextDay } from '@dereekb/util';
import { DayOfWeek, RequiredOnKeys, IndexNumber, IndexRange, indexRangeCheckFunction, IndexRef, MINUTES_IN_DAY, MS_IN_DAY, UniqueModel, lastValue, FactoryWithRequiredInput, FilterFunction, mergeFilterFunctions, range, Milliseconds, Hours, MapFunction, getNextDay, SortCompareFunction, sortAscendingIndexNumberRefFunction } from '@dereekb/util';
import { dateRange, DateRange, DateRangeDayDistanceInput, DateRangeType, isDateRange } from './date.range';
import { DateDurationSpan } from './date.duration';
import { differenceInDays, differenceInMilliseconds, isBefore, addDays, addMinutes, setSeconds, addMilliseconds, hoursToMilliseconds, addHours } from 'date-fns';
Expand Down Expand Up @@ -451,6 +451,27 @@ export function dateBlockRange(i: number, to?: number): DateBlockRangeWithRange
return { i, to: to ?? i };
}

/**
* Sorts the input ranges by index and distance (to values).
*
* In many cases sortAscendingIndexNumberRefFunction may be preferential since
*
* @returns
*/
export function sortDateBlockRangeAndSizeFunction<T extends DateBlockRange>(): SortCompareFunction<T> {
return (a, b) => a.i - b.i || (a.to ?? a.i) - (b.to ?? b.i);
}

/**
* Sorts the input date ranges. This will retain the before/after order while also sorting items by index.
*
* @param input
* @returns
*/
export function sortDateBlockRanges<T extends DateBlockRange>(input: T[]): T[] {
return input.sort(sortAscendingIndexNumberRefFunction());
}

/**
* DateBlockRange that is known to have a to value.
*/
Expand All @@ -467,7 +488,7 @@ export function groupToDateBlockRanges(input: (DateBlock | DateBlockRange)[]): D
}

// sort by index in ascending order
const blocks = input.sort(sortAscendingIndexNumberRefFunction());
const blocks = sortDateBlockRanges(input);

function newBlockFromBlocksIndex(blocksIndex: number): DateBlockRangeWithRange {
const { i, to } = blocks[blocksIndex] as DateBlockRange;
Expand Down Expand Up @@ -553,10 +574,10 @@ export interface UniqueDateBlockRangeGroup<B extends DateBlockRange | UniqueDate
}

/**
* Groups all input DateBlockRange or UniqueDateBlock values into a UniqueDateBlockRangeGroup value.
* Groups all input DateBlockRange or UniqueDateBlock values into a UniqueDateBlockRangeGroup value amd sorts the input.
*/
export function groupUniqueDateBlocks<B extends DateBlockRange | UniqueDateBlock>(input: B[]): UniqueDateBlockRangeGroup<B> {
const blocks = [...input].sort(sortAscendingIndexNumberRefFunction());
const blocks = sortDateBlockRanges([...input]);

const i = 0;
let to: number;
Expand Down Expand Up @@ -662,7 +683,7 @@ export function expandUniqueDateBlocks<B extends DateBlockRange | UniqueDateBloc
addGapBlock(gapStartIndex, i - 1);
}

const to = Math.min(inputTo, maxAllowedIndex);
const to = Math.min(inputTo, maxAllowedIndex) || 0;

const block: B = {
...inputBlock,
Expand Down Expand Up @@ -694,7 +715,7 @@ export function expandUniqueDateBlocks<B extends DateBlockRange | UniqueDateBloc
};

const block: B = fillFactory(dateBlockRange);
addBlockWithRange(block, i, to);
addBlockWithRange(block, i, to ?? i);
} else if (blocks.length > 0) {
// do not extend if no blocks have been pushed.
const blockToExtend = lastValue(blocks);
Expand Down Expand Up @@ -776,6 +797,19 @@ export function expandUniqueDateBlocks<B extends DateBlockRange | UniqueDateBloc
addBlockWithRange(current, currentNextIndex, currentEndIndex);
// change next to start at the next range
continueToNext({ ...next, i: currentEndIndex + 1, to: nextEndIndex });
} else {
// the next item ends before the current item.
if (overwriteOption === 'current') {
// add the next item first since it overwrites the current
addBlockWithRange(next, nextStartIndex, nextEndIndex);
// continue with the current item as next.
continueToNext({ ...current, i: nextEndIndex + 1, to: currentEndIndex });
} else {
// discard the next value.
discard(next);
// continue with the current value
continueToNext(current);
}
}
} else {
// Check for any overlap
Expand All @@ -796,7 +830,6 @@ export function expandUniqueDateBlocks<B extends DateBlockRange | UniqueDateBloc
} else {
// add the block
addBlockWithRange(current, currentNextIndex, currentEndIndex);

// continue to next
continueToNext();
}
Expand All @@ -805,7 +838,6 @@ export function expandUniqueDateBlocks<B extends DateBlockRange | UniqueDateBloc

if (current != null) {
// if current != null, then atleast one block was input/remaining.

const lastStartIndex = current.i;
const lastEndIndex = dateBlockEndIndex(current);

Expand Down

0 comments on commit 7464f2d

Please sign in to comment.