Skip to content

Commit

Permalink
refactor(range): use standard format & observe inclusive range bounds (
Browse files Browse the repository at this point in the history
  • Loading branch information
mjy78 authored and sushantdhiman committed May 7, 2018
1 parent bd6fa21 commit b66f9dd
Show file tree
Hide file tree
Showing 9 changed files with 204 additions and 146 deletions.
13 changes: 2 additions & 11 deletions docs/models-definition.md
Expand Up @@ -220,13 +220,6 @@ When supplying ranges as values you can choose from the following APIs:
Timeline.create({ range: [new Date(Date.UTC(2016, 0, 1)), new Date(Date.UTC(2016, 1, 1))] });

// control inclusion
const range = [new Date(Date.UTC(2016, 0, 1)), new Date(Date.UTC(2016, 1, 1))];
range.inclusive = false; // '()'
range.inclusive = [false, true]; // '(]'
range.inclusive = true; // '[]'
range.inclusive = [true, false]; // '[)'

// or as a single expression
const range = [
{ value: new Date(Date.UTC(2016, 0, 1)), inclusive: false },
{ value: new Date(Date.UTC(2016, 1, 1)), inclusive: true },
Expand All @@ -248,12 +241,10 @@ receive:

```js
// stored value: ("2016-01-01 00:00:00+00:00", "2016-02-01 00:00:00+00:00"]
range // [Date, Date]
range.inclusive // [false, true]
range // [{ value: Date, inclusive: false }, { value: Date, inclusive: true }]
```

Make sure you turn that into a serializable format before serialization since array
extra properties will not be serialized.
You will need to call reload after updating an instance with a range type or use `returning: true` option.

**Special Cases**

Expand Down
4 changes: 0 additions & 4 deletions lib/data-types.js
Expand Up @@ -571,10 +571,6 @@ RANGE.prototype.toCastType = function toCastType() {
return pgRangeCastTypes[this._subtype.toLowerCase()];
};
RANGE.prototype.validate = function validate(value) {
if (_.isPlainObject(value) && value.inclusive) {
value = value.inclusive;
}

if (!_.isArray(value)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid range', value));
}
Expand Down
12 changes: 10 additions & 2 deletions lib/dialects/postgres/data-types.js
Expand Up @@ -538,7 +538,15 @@ module.exports = BaseTypes => {
return "'" + this.options.subtype.stringify(values, options) + "'::" +
this.toCastType();
}
const valuesStringified = values.map(value => {

const valueInclusivity = [true, false];
const valuesStringified = values.map((value, index) => {
if (_.isObject(value) && value.hasOwnProperty('value')) {
if (value.hasOwnProperty('inclusive')) {
valueInclusivity[index] = value.inclusive;
}
value = value.value;
}
if (_.includes([null, -Infinity, Infinity], value)) {
// Pass through "unbounded" bounds unchanged
return value;
Expand All @@ -550,7 +558,7 @@ module.exports = BaseTypes => {
});

// Array.map does not preserve extra array properties
valuesStringified.inclusive = values.inclusive;
valuesStringified.inclusive = valueInclusivity;

return '\'' + range.stringify(valuesStringified) + '\'';
};
Expand Down
13 changes: 7 additions & 6 deletions lib/dialects/postgres/range.js
Expand Up @@ -56,9 +56,7 @@ exports.stringify = stringify;
function parse(value, parser) {
if (value === null) return null;
if (value === 'empty') {
const empty = [];
empty.inclusive = [];
return empty;
return [];
}

let result = value
Expand All @@ -67,9 +65,12 @@ function parse(value, parser) {

if (result.length !== 2) return value;

result = result.map(value => parseRangeBound(value, parser));

result.inclusive = [value[0] === '[', value[value.length - 1] === ']'];
result = result.map((item, index) => {
return {
value: parseRangeBound(item, parser),
inclusive: index === 0 ? value[0] === '[' : value[value.length - 1] === ']'
};
});

return result;
}
Expand Down
6 changes: 2 additions & 4 deletions lib/utils.js
Expand Up @@ -208,12 +208,10 @@ function mapWhereFieldNames(attributes, Model) {
}

if (Array.isArray(attributes[attribute])) {
attributes[attribute] = attributes[attribute].map(where => {
_.forEach(attributes[attribute], (where, index) => {
if (_.isPlainObject(where)) {
return mapWhereFieldNames(where, Model);
attributes[attribute][index] = mapWhereFieldNames(where, Model);
}

return where;
});
}

Expand Down
105 changes: 103 additions & 2 deletions test/integration/data-types.test.js
Expand Up @@ -8,6 +8,7 @@ const chai = require('chai'),
_ = require('lodash'),
moment = require('moment'),
current = Support.sequelize,
Op = current.Op,
uuid = require('uuid'),
DataTypes = require('../../lib/data-types'),
dialect = Support.getTestDialect(),
Expand Down Expand Up @@ -436,8 +437,108 @@ describe(Support.getTestDialectTeaser('DataTypes'), () => {
.then(() => Model.create({ interval: [1, 4] }) )
.then(() => Model.findAll() )
.spread(m => {
expect(m.interval[0]).to.be.eql(1);
expect(m.interval[1]).to.be.eql(4);
expect(m.interval[0].value).to.be.eql(1);
expect(m.interval[1].value).to.be.eql(4);
});
});
}

if (current.dialect.supports.RANGE) {

it('should allow date ranges to be generated with default bounds inclusion #8176', function() {
const Model = this.sequelize.define('M', {
interval: {
type: Sequelize.RANGE(Sequelize.DATE),
allowNull: false,
unique: true
}
});
const testDate1 = new Date();
const testDate2 = new Date(testDate1.getTime() + 10000);
const testDateRange = [testDate1, testDate2];

return Model.sync({ force: true })
.then(() => Model.create({ interval: testDateRange }))
.then(() => Model.findOne())
.then(m => {
expect(m).to.exist;
expect(m.interval[0].value).to.be.eql(testDate1);
expect(m.interval[1].value).to.be.eql(testDate2);
expect(m.interval[0].inclusive).to.be.eql(true);
expect(m.interval[1].inclusive).to.be.eql(false);
});
});

it('should allow date ranges to be generated using a single range expression to define bounds inclusion #8176', function() {
const Model = this.sequelize.define('M', {
interval: {
type: Sequelize.RANGE(Sequelize.DATE),
allowNull: false,
unique: true
}
});
const testDate1 = new Date();
const testDate2 = new Date(testDate1.getTime() + 10000);
const testDateRange = [{ value: testDate1, inclusive: false }, { value: testDate2, inclusive: true }];

return Model.sync({ force: true })
.then(() => Model.create({ interval: testDateRange }))
.then(() => Model.findOne())
.then(m => {
expect(m).to.exist;
expect(m.interval[0].value).to.be.eql(testDate1);
expect(m.interval[1].value).to.be.eql(testDate2);
expect(m.interval[0].inclusive).to.be.eql(false);
expect(m.interval[1].inclusive).to.be.eql(true);
});
});

it('should allow date ranges to be generated using a composite range expression #8176', function() {
const Model = this.sequelize.define('M', {
interval: {
type: Sequelize.RANGE(Sequelize.DATE),
allowNull: false,
unique: true
}
});
const testDate1 = new Date();
const testDate2 = new Date(testDate1.getTime() + 10000);
const testDateRange = [testDate1, { value: testDate2, inclusive: true }];

return Model.sync({ force: true })
.then(() => Model.create({ interval: testDateRange }))
.then(() => Model.findOne())
.then(m => {
expect(m).to.exist;
expect(m.interval[0].value).to.be.eql(testDate1);
expect(m.interval[1].value).to.be.eql(testDate2);
expect(m.interval[0].inclusive).to.be.eql(true);
expect(m.interval[1].inclusive).to.be.eql(true);
});
});

it('should correctly return ranges when using predicates that define bounds inclusion #8176', function() {
const Model = this.sequelize.define('M', {
interval: {
type: Sequelize.RANGE(Sequelize.DATE),
allowNull: false,
unique: true
}
});
const testDate1 = new Date();
const testDate2 = new Date(testDate1.getTime() + 10000);
const testDateRange = [testDate1, testDate2];
const dateRangePredicate = [{ value: testDate1, inclusive: true }, { value: testDate1, inclusive: true }];

return Model.sync({ force: true })
.then(() => Model.create({ interval: testDateRange }))
.then(() => Model.findOne({
where: {
interval: { [Op.overlap]: dateRangePredicate }
}
}))
.then(m => {
expect(m).to.exist;
});
});
}
Expand Down

0 comments on commit b66f9dd

Please sign in to comment.