Skip to content

Commit

Permalink
fix(Set): replace override option with merge
Browse files Browse the repository at this point in the history
The override option has now been replaced with the merge option to be more consistent with cypher.
The merge option is just the opposite of the override option. Setting to true will cause the `+=`
operator to be used in the query.

This also affects the default behaviour of the Set class and all `.set*` family functions. There
was previously an inconsistency between the Set class and the set methods on the builder interface
in regards to whether to use the `=` or `+=` operator by default which has now been normalized to
always use `=`. This means that the new merge option will always default to false.

BREAKING CHANGE: The default behaviour of the Set clause has changed to use the `=` operator.
This is to be more consistent with cypher.
  • Loading branch information
jamesfer committed Sep 20, 2019
1 parent 5ead489 commit 91ab4f6
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 40 deletions.
40 changes: 21 additions & 19 deletions src/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,16 +119,17 @@ export class SetBlock<Q> {
* `setValues` accepts a dictionary where the keys are nodes or property names
* to be updated.
*
* `setValues` and `setVariables` by default set override to false meaning
* that the `+=` operator will be used to merge values. You can set override
* to `true` to use the `=` operator instead.
*
* @param {_.Dictionary<any>} values
* @param {boolean} override
* @returns {Q}
* To use the `+=` operator to merge properties of a node, you can pass
* `true` to the merge option.
* ```
* query.setValues({
* 'sale': { active: true },
* }, true)
* // SET sale += $sale
* ```
*/
setValues(values: Dictionary<any>, override?: boolean) {
return this.chain(this.wrap(new Set({ values }, { override })));
setValues(values: Dictionary<any>, merge?: boolean) {
return this.chain(this.wrap(new Set({ values }, { merge })));
}

/**
Expand All @@ -142,20 +143,21 @@ export class SetBlock<Q> {
* query.setVariables({
* 'sale.activatedAt': 'timestamp()',
* })
* // SET sale.activatedAt += timestamp()
* // SET sale.activatedAt = timestamp()
* ```
* Note how values are inserted into the query, as is.
*
* `setValues` and `setVariables` by default set override to false meaning
* that the `+=` operator will be used to merge values. You can set override
* to `true` to use the `=` operator instead.
*
* @param {_.Dictionary<string | _.Dictionary<string>>} variables
* @param {boolean} override
* @returns {Q}
* To use the `+=` operator to merge properties of a node, you can pass
* `true` to the merge option.
* ```
* query.setVariables({
* 'sale': 'newSaleDetails'
* }, true)
* // SET sale += newSaleDetails
* ```
*/
setVariables(variables: Dictionary<string | Dictionary<string>>, override?: boolean) {
return this.chain(this.wrap(new Set({ variables }, { override })));
setVariables(variables: Dictionary<string | Dictionary<string>>, merge?: boolean) {
return this.chain(this.wrap(new Set({ variables }, { merge })));
}

private wrap(clause: Set): Clause {
Expand Down
12 changes: 6 additions & 6 deletions src/clauses/set.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,20 +57,20 @@ describe('Set', () => {
expect(queryParams).to.have.property('node2', param2);
});

it('should merge properties when override is false', () => {
it('should merge properties when merge is true', () => {
const data = {
values: { node: { name: 'complex value' } },
variables: { node2: 'variable' },
};
const query = new Set(data, { override: false });
const query = new Set(data, { merge: true });
expect(query.build()).to.equal('SET node += $node, node2 += variable');
});

it('should not merge plain values even when override is false', () => {
it('should not merge plain values even when merge is true', () => {
const data = {
values: { node: 'value' },
values: { 'node.property': 'value', otherNode: { dictionary: 'value' } },
};
const query = new Set(data, { override: false });
expect(query.build()).to.equal('SET node = $node');
const query = new Set(data, { merge: true });
expect(query.build()).to.equal('SET node.property = $nodeProperty, otherNode += $otherNode');
});
});
19 changes: 6 additions & 13 deletions src/clauses/set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,27 @@ export type SetProperties = {
};

export interface SetOptions {
override?: boolean;
merge?: boolean;
}

export class Set extends Clause {
protected labels: Dictionary<string[]>;
protected values: Dictionary<Parameter>;
protected variables: Dictionary<string | Dictionary<string>>;
protected override: boolean;
protected merge: boolean;

protected makeLabelStatement = (labels: Many<string>, key: string) => {
return key + stringifyLabels(labels);
}

protected makeValueStatement = (value: any, key: string): string => {
const valueIsObject = value instanceof Parameter ? isObject(value.value) : isObject(value);
const op = this.override || !valueIsObject ? ' = ' : ' += ';
const op = this.merge && valueIsObject ? ' += ' : ' = ';
return key + op + value;
}

protected makeVariableStatement = (value: string | Dictionary<string>, key: string): string => {
const op = this.override ? ' = ' : ' += ';
const op = this.merge ? ' += ' : ' = ';
if (isString(value)) {
return key + op + value;
}
Expand All @@ -43,23 +43,16 @@ export class Set extends Clause {

constructor(
{ labels, values, variables }: SetProperties,
inOptions?: SetOptions,
options: SetOptions = {},
) {
super();

let options: SetOptions | undefined = inOptions;
if (options === undefined) {
// tslint:disable-next-line:max-line-length
console.warn('Warning: In the future, override will default to false in a Set clause when no options are provided. To retain the old behaviour, pass { override: false } as options to the Set constructor.');
options = { override: true };
}

this.labels = mapValues(labels, castArray);
this.values = mapValues(values, (value, name) => {
return this.parameterBag.addParam(value, name);
});
this.variables = variables || {};
this.override = options.override === undefined ? false : options.override;
this.merge = !!options.merge;
}

build() {
Expand Down
2 changes: 1 addition & 1 deletion src/query.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('Query', () => {
orderBy: q => q.orderBy('name'),
raw: q => q.raw('name'),
return: q => q.return('node'),
set: q => q.set({}, { override: false }),
set: q => q.set({}, { merge: false }),
setLabels: q => q.setLabels({}),
setValues: q => q.setValues({}),
setVariables: q => q.setVariables({}, false),
Expand Down
2 changes: 1 addition & 1 deletion tests/connection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ describe('Connection', () => {
unwind: () => connection.unwind([1, 2, 3], 'number'),
delete: () => connection.delete('node'),
detachDelete: () => connection.detachDelete('node'),
set: () => connection.set({}, { override: false }),
set: () => connection.set({}, { merge: false }),
setLabels: () => connection.setLabels({}),
setValues: () => connection.setValues({}),
setVariables: () => connection.setVariables({}, false),
Expand Down

1 comment on commit 91ab4f6

@nikolasstow
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SetOptions's override was renamed merge however the docs were not updated with this modification.

Please sign in to comment.