Skip to content

public / sequelize

Go to a project

不要怂，就是干，撸起袖子干！

Commit b66f9ddc by Michael Yates Committed by Sushant
Options

refactor(range): use standard format & observe inclusive range bounds (#9364)

1 parent bd6fa212
Expand all
Inline Side-by-side
Showing with 137 additions and 86 deletions
docs/models-definition.md
...@@ -220,13 +220,6 @@ When supplying ranges as values you can choose from the following APIs: ...@@ -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))] }); Timeline.create({ range: [new Date(Date.UTC(2016, 0, 1)), new Date(Date.UTC(2016, 1, 1))] });
// control inclusion // 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 = [ const range = [
{ value: new Date(Date.UTC(2016, 0, 1)), inclusive: false }, { value: new Date(Date.UTC(2016, 0, 1)), inclusive: false },
{ value: new Date(Date.UTC(2016, 1, 1)), inclusive: true }, { value: new Date(Date.UTC(2016, 1, 1)), inclusive: true },
...@@ -248,12 +241,10 @@ receive: ...@@ -248,12 +241,10 @@ receive:
```js ```js
// stored value: ("2016-01-01 00:00:00+00:00", "2016-02-01 00:00:00+00:00"] // stored value: ("2016-01-01 00:00:00+00:00", "2016-02-01 00:00:00+00:00"]
range // [Date, Date] range // [{ value: Date, inclusive: false }, { value: Date, inclusive: true }]
range.inclusive // [false, true]
``` ```
Make sure you turn that into a serializable format before serialization since array You will need to call reload after updating an instance with a range type or use `returning: true` option.
extra properties will not be serialized.
**Special Cases** **Special Cases**
......
lib/data-types.js
...@@ -571,10 +571,6 @@ RANGE.prototype.toCastType = function toCastType() { ...@@ -571,10 +571,6 @@ RANGE.prototype.toCastType = function toCastType() {
return pgRangeCastTypes[this._subtype.toLowerCase()]; return pgRangeCastTypes[this._subtype.toLowerCase()];
}; };
RANGE.prototype.validate = function validate(value) { RANGE.prototype.validate = function validate(value) {
if (_.isPlainObject(value) && value.inclusive) {
value = value.inclusive;
}
if (!_.isArray(value)) { if (!_.isArray(value)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid range', value)); throw new sequelizeErrors.ValidationError(util.format('%j is not a valid range', value));
} }
......
lib/dialects/postgres/data-types.js
...@@ -538,7 +538,15 @@ module.exports = BaseTypes => { ...@@ -538,7 +538,15 @@ module.exports = BaseTypes => {
return "'" + this.options.subtype.stringify(values, options) + "'::" + return "'" + this.options.subtype.stringify(values, options) + "'::" +
this.toCastType(); 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)) { if (_.includes([null, -Infinity, Infinity], value)) {
// Pass through "unbounded" bounds unchanged // Pass through "unbounded" bounds unchanged
return value; return value;
...@@ -550,7 +558,7 @@ module.exports = BaseTypes => { ...@@ -550,7 +558,7 @@ module.exports = BaseTypes => {
}); });
// Array.map does not preserve extra array properties // Array.map does not preserve extra array properties
valuesStringified.inclusive = values.inclusive; valuesStringified.inclusive = valueInclusivity;
return '\'' + range.stringify(valuesStringified) + '\''; return '\'' + range.stringify(valuesStringified) + '\'';
}; };
......
lib/dialects/postgres/range.js
...@@ -56,9 +56,7 @@ exports.stringify = stringify; ...@@ -56,9 +56,7 @@ exports.stringify = stringify;
function parse(value, parser) { function parse(value, parser) {
if (value === null) return null; if (value === null) return null;
if (value === 'empty') { if (value === 'empty') {
const empty = []; return [];
empty.inclusive = [];
return empty;
} }
let result = value let result = value
...@@ -67,9 +65,12 @@ function parse(value, parser) { ...@@ -67,9 +65,12 @@ function parse(value, parser) {
if (result.length !== 2) return value; if (result.length !== 2) return value;
result = result.map(value => parseRangeBound(value, parser)); result = result.map((item, index) => {
return {
result.inclusive = [value[0] === '[', value[value.length - 1] === ']']; value: parseRangeBound(item, parser),
inclusive: index === 0 ? value[0] === '[' : value[value.length - 1] === ']'
};
});
return result; return result;
} }
......
lib/utils.js
...@@ -208,12 +208,10 @@ function mapWhereFieldNames(attributes, Model) { ...@@ -208,12 +208,10 @@ function mapWhereFieldNames(attributes, Model) {
} }
if (Array.isArray(attributes[attribute])) { if (Array.isArray(attributes[attribute])) {
attributes[attribute] = attributes[attribute].map(where => { _.forEach(attributes[attribute], (where, index) => {
if (_.isPlainObject(where)) { if (_.isPlainObject(where)) {
return mapWhereFieldNames(where, Model); attributes[attribute][index] = mapWhereFieldNames(where, Model);
} }
return where;
}); });
} }
......
test/integration/data-types.test.js
...@@ -8,6 +8,7 @@ const chai = require('chai'), ...@@ -8,6 +8,7 @@ const chai = require('chai'),
_ = require('lodash'), _ = require('lodash'),
moment = require('moment'), moment = require('moment'),
current = Support.sequelize, current = Support.sequelize,
Op = current.Op,
uuid = require('uuid'), uuid = require('uuid'),
DataTypes = require('../../lib/data-types'), DataTypes = require('../../lib/data-types'),
dialect = Support.getTestDialect(), dialect = Support.getTestDialect(),
...@@ -436,8 +437,108 @@ describe(Support.getTestDialectTeaser('DataTypes'), () => { ...@@ -436,8 +437,108 @@ describe(Support.getTestDialectTeaser('DataTypes'), () => {
.then(() => Model.create({ interval: [1, 4] }) ) .then(() => Model.create({ interval: [1, 4] }) )
.then(() => Model.findAll() ) .then(() => Model.findAll() )
.spread(m => { .spread(m => {
expect(m.interval[0]).to.be.eql(1); expect(m.interval[0].value).to.be.eql(1);
expect(m.interval[1]).to.be.eql(4); 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;
}); });
}); });
} }
......
test/integration/dialects/postgres/range.test.js
...@@ -5,9 +5,8 @@ const chai = require('chai'), ...@@ -5,9 +5,8 @@ const chai = require('chai'),
Support = require(__dirname + '/../../support'), Support = require(__dirname + '/../../support'),
DataTypes = require(__dirname + '/../../../../lib/data-types'), DataTypes = require(__dirname + '/../../../../lib/data-types'),
dialect = Support.getTestDialect(), dialect = Support.getTestDialect(),
range = require('../../../../lib/dialects/postgres/range'), range = require('../../../../lib/dialects/postgres/range');
_ = require('lodash');
if (dialect.match(/^postgres/)) { if (dialect.match(/^postgres/)) {
// Don't try to load pg until we know we're running on postgres. // Don't try to load pg until we know we're running on postgres.
const pg = require('pg'); const pg = require('pg');
...@@ -50,25 +49,6 @@ if (dialect.match(/^postgres/)) { ...@@ -50,25 +49,6 @@ if (dialect.match(/^postgres/)) {
expect(range.stringify([0, { inclusive: true, value: 1 }])).to.equal('[0,1]'); expect(range.stringify([0, { inclusive: true, value: 1 }])).to.equal('[0,1]');
}); });
it('should handle inclusive property of input array properly', () => {
const testRange = [1, 2];
testRange.inclusive = [true, false];
expect(range.stringify(testRange)).to.equal('[1,2)');
testRange.inclusive = [false, true];
expect(range.stringify(testRange)).to.equal('(1,2]');
testRange.inclusive = [true, true];
expect(range.stringify(testRange)).to.equal('[1,2]');
testRange.inclusive = true;
expect(range.stringify(testRange)).to.equal('[1,2]');
testRange.inclusive = false;
expect(range.stringify(testRange)).to.equal('(1,2)');
});
it('should handle date values', () => { it('should handle date values', () => {
const Range = new DataTypes.postgres.RANGE(DataTypes.DATE); const Range = new DataTypes.postgres.RANGE(DataTypes.DATE);
expect(Range.stringify([new Date(Date.UTC(2000, 1, 1)), expect(Range.stringify([new Date(Date.UTC(2000, 1, 1)),
...@@ -183,21 +163,21 @@ if (dialect.match(/^postgres/)) { ...@@ -183,21 +163,21 @@ if (dialect.match(/^postgres/)) {
}); });
it('should handle empty range string correctly', () => { it('should handle empty range string correctly', () => {
expect(range.parse('empty')).to.deep.equal(_.extend([], { inclusive: [] })); expect(range.parse('empty')).to.deep.equal([]);
}); });
it('should handle empty bounds correctly', () => { it('should handle empty bounds correctly', () => {
expect(range.parse('(1,)', DataTypes.postgres.INTEGER.parse)).to.deep.equal(_.extend([1, null], { inclusive: [false, false] })); expect(range.parse('(1,)', DataTypes.postgres.INTEGER.parse)).to.deep.equal([{ value: 1, inclusive: false }, { value: null, inclusive: false }]);
expect(range.parse('(,1)', DataTypes.postgres.INTEGER.parse)).to.deep.equal(_.extend([null, 1], { inclusive: [false, false] })); expect(range.parse('(,1)', DataTypes.postgres.INTEGER.parse)).to.deep.equal([{ value: null, inclusive: false }, { value: 1, inclusive: false }]);
expect(range.parse('(,)', DataTypes.postgres.INTEGER.parse)).to.deep.equal(_.extend([null, null], { inclusive: [false, false] })); expect(range.parse('(,)', DataTypes.postgres.INTEGER.parse)).to.deep.equal([{ value: null, inclusive: false }, { value: null, inclusive: false }]);
}); });
it('should handle infinity/-infinity bounds correctly', () => { it('should handle infinity/-infinity bounds correctly', () => {
expect(range.parse('(infinity,1)', DataTypes.postgres.INTEGER.parse)).to.deep.equal(_.extend([Infinity, 1], { inclusive: [false, false] })); expect(range.parse('(infinity,1)', DataTypes.postgres.INTEGER.parse)).to.deep.equal([{ value: Infinity, inclusive: false }, { value: 1, inclusive: false }]);
expect(range.parse('(1,infinity)', DataTypes.postgres.INTEGER.parse)).to.deep.equal(_.extend([1, Infinity], { inclusive: [false, false] })); expect(range.parse('(1,infinity)', DataTypes.postgres.INTEGER.parse)).to.deep.equal([{ value: 1, inclusive: false }, { value: Infinity, inclusive: false }]);
expect(range.parse('(-infinity,1)', DataTypes.postgres.INTEGER.parse)).to.deep.equal(_.extend([-Infinity, 1], { inclusive: [false, false] })); expect(range.parse('(-infinity,1)', DataTypes.postgres.INTEGER.parse)).to.deep.equal([{ value: -Infinity, inclusive: false }, { value: 1, inclusive: false }]);
expect(range.parse('(1,-infinity)', DataTypes.postgres.INTEGER.parse)).to.deep.equal(_.extend([1, -Infinity], { inclusive: [false, false] })); expect(range.parse('(1,-infinity)', DataTypes.postgres.INTEGER.parse)).to.deep.equal([{ value: 1, inclusive: false }, { value: -Infinity, inclusive: false }]);
expect(range.parse('(-infinity,infinity)', DataTypes.postgres.INTEGER.parse)).to.deep.equal(_.extend([-Infinity, Infinity], { inclusive: [false, false] })); expect(range.parse('(-infinity,infinity)', DataTypes.postgres.INTEGER.parse)).to.deep.equal([{ value: -Infinity, inclusive: false }, { value: Infinity, inclusive: false }]);
}); });
it('should return raw value if not range is returned', () => { it('should return raw value if not range is returned', () => {
...@@ -207,15 +187,13 @@ if (dialect.match(/^postgres/)) { ...@@ -207,15 +187,13 @@ if (dialect.match(/^postgres/)) {
it('should handle native postgres timestamp format', () => { it('should handle native postgres timestamp format', () => {
const tsOid = DataTypes.postgres.DATE.types.postgres.oids[0], const tsOid = DataTypes.postgres.DATE.types.postgres.oids[0],
parser = pg.types.getTypeParser(tsOid); parser = pg.types.getTypeParser(tsOid);
expect(range.parse('(2016-01-01 08:00:00-04,)', parser)[0].toISOString()).to.equal('2016-01-01T12:00:00.000Z'); expect(range.parse('(2016-01-01 08:00:00-04,)', parser)[0].value.toISOString()).to.equal('2016-01-01T12:00:00.000Z');
}); });
}); });
describe('stringify and parse', () => { describe('stringify and parse', () => {
it('should stringify then parse back the same structure', () => { it('should stringify then parse back the same structure', () => {
const testRange = [5, 10]; const testRange = [{ value: 5, inclusive: true }, { value: 10, inclusive: true }];
testRange.inclusive = [true, true];
const Range = new DataTypes.postgres.RANGE(DataTypes.INTEGER); const Range = new DataTypes.postgres.RANGE(DataTypes.INTEGER);
let stringified = Range.stringify(testRange, {}); let stringified = Range.stringify(testRange, {});
......
test/unit/sql/data-types.test.js
...@@ -1296,33 +1296,11 @@ suite(Support.getTestDialectTeaser('SQL'), () => { ...@@ -1296,33 +1296,11 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
}).to.throw(Sequelize.ValidationError, 'A range must be an array with two elements'); }).to.throw(Sequelize.ValidationError, 'A range must be an array with two elements');
}); });
test('should throw an error if `value.inclusive` is invalid', () => {
const type = DataTypes.RANGE();
expect(() => {
type.validate({ inclusive: 'foobar' });
}).to.throw(Sequelize.ValidationError, '"foobar" is not a valid range');
});
test('should throw an error if `value.inclusive` is not an array with two elements', () => {
const type = DataTypes.RANGE();
expect(() => {
type.validate({ inclusive: [1] });
}).to.throw(Sequelize.ValidationError, 'A range must be an array with two elements');
});
test('should return `true` if `value` is a range', () => { test('should return `true` if `value` is a range', () => {
const type = DataTypes.RANGE(); const type = DataTypes.RANGE();
expect(type.validate([1, 2])).to.equal(true); expect(type.validate([1, 2])).to.equal(true);
}); });
test('should return `true` if `value.inclusive` is a range', () => {
const type = DataTypes.RANGE();
expect(type.validate({ inclusive: [1, 2] })).to.equal(true);
});
}); });
}); });
......
Markdown is supported
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!