Skip to content

Commit

Permalink
fix(termlist): fix handling of nested dictionaries
Browse files Browse the repository at this point in the history
The documentation described the behaviour of `with` and `return` when given a nested set of
dictionaries:
```javascript
query.with({
  people: {
    name: 'personName',
    age: 'personAge',
  },
});
// WITH people.name AS personName, people.age AS personAge
```

However the types forbade this. If using Javascript, no error was returned, however the
result was very different to the documentation. It now behaves as documentated.

If you are using Typescript this is not a breaking change as the types would have
prevented you from using the faulty behaviour.

If you as using Javascript then there is a chance that this will change the behaviour
of your code but it is unlikely and the other behaviour is a bug.

fixes #137
  • Loading branch information
jamesfer committed Dec 12, 2020
1 parent c712161 commit 93a5cd4
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 27 deletions.
8 changes: 4 additions & 4 deletions src/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ export abstract class Builder<Q> extends SetBlock<Q> {
* query.return({
* people: [{ name: 'personName' }, 'age' ],
* })
* // RETURN people.name as personName, people.age
* // RETURN people.name AS personName, people.age
* ```
* or
* ```javascript
Expand All @@ -636,7 +636,7 @@ export abstract class Builder<Q> extends SetBlock<Q> {
* age: 'personAge',
* },
* })
* // RETURN people.name as personName, people.age as personAge
* // RETURN people.name AS personName, people.age AS personAge
* ```
*
* You can also pass an array of any of the above methods.
Expand Down Expand Up @@ -895,7 +895,7 @@ export abstract class Builder<Q> extends SetBlock<Q> {
* query.with({
* people: [{ name: 'personName' }, 'age' ],
* })
* // WITH people.name as personName, people.age
* // WITH people.name AS personName, people.age
* ```
* or
* ```javascript
Expand All @@ -905,7 +905,7 @@ export abstract class Builder<Q> extends SetBlock<Q> {
* age: 'personAge',
* },
* })
* // WITH people.name as personName, people.age as personAge
* // WITH people.name AS personName, people.age AS personAge
* ```
*
* You can also pass an array of any of the above methods.
Expand Down
6 changes: 6 additions & 0 deletions src/clauses/term-list-clause.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ describe('TermListClause', () => {
expect(termList.getParams()).to.be.empty;
});

it('should rename a single nested object property', () => {
const termList = new TermListClause({ node: { name: 'otherName', timestamp: 'created_at' } });
expect(termList.build()).to.equal('node.name AS otherName, node.timestamp AS created_at');
expect(termList.getParams()).to.be.empty;
});

it('should be able to apply functions to properties', () => {
const termList = new TermListClause('avg(node.count)');
expect(termList.build()).to.equal('avg(node.count)');
Expand Down
34 changes: 13 additions & 21 deletions src/clauses/term-list-clause.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,9 @@ import {
} from 'lodash';
import { Clause } from '../clause';

export type Properties = (string | Dictionary<string>)[];
export type Properties = Many<string | Dictionary<string>>;
export type Term
= string
| Dictionary<string>
| Dictionary<Properties>;

export class TermListClause extends Clause {
Expand All @@ -34,35 +33,32 @@ export class TermListClause extends Clause {
this.terms = castArray(terms);
}

build() {
return this.toString();
}

toString() {
return flatMapDeep(this.terms, term => this.stringifyTerm(term)).join(', ');
}

private stringifyTerm(term: Term): Many<string> {
private stringifyTerm(term: Term): string[] {
// Just a node
if (isString(term)) {
return this.stringifyProperty(term);
}

// List of nodes
if (isArray(term)) {
return this.stringifyProperties(term);
return [this.stringifyProperty(term)];
}

// Node properties or aliases
if (isPlainObject(term)) {
return this.stringifyDictionary(term);
}

return '';
return [];
}

private stringifyProperty(prop: string, alias?: string, node?: string): string {
let prefix = node ? `${node}.` : '';
if (alias) {
prefix += `${alias} AS `;
}
return prefix + prop;
const prefixString = node ? `${node}.` : '';
const aliasString = alias ? `${alias} AS ` : '';
return prefixString + aliasString + prop;
}

private stringifyProperties(props: Properties, alias?: string, node?: string): string[] {
Expand All @@ -76,10 +72,10 @@ export class TermListClause extends Clause {
}
return list;
};
return reduce(props, convertToString, []);
return reduce(castArray(props), convertToString, []);
}

private stringifyDictionary(node: Dictionary<string | Properties>): string[] {
private stringifyDictionary(node: Dictionary<Properties>): string[] {
return reduce(
node,
(list, prop, key) => {
Expand All @@ -95,8 +91,4 @@ export class TermListClause extends Clause {
[] as string[],
);
}

build() {
return this.toString();
}
}
3 changes: 1 addition & 2 deletions tests/utils.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import { Connection, Credentials } from '../src';
import IHookCallbackContext = Mocha.IHookCallbackContext;

export const neo4jUrl: string = process.env.NEO4J_URL as string;
export const neo4jCredentials: Credentials = {
username: process.env.NEO4J_USER as string,
password: process.env.NEO4J_PASS as string,
};

export async function waitForNeo(this: IHookCallbackContext) {
export async function waitForNeo(this: Mocha.Context) {
if (this && 'timeout' in this) {
this.timeout(40000);
}
Expand Down

0 comments on commit 93a5cd4

Please sign in to comment.