Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for duplicate fieldnames in dom.Value#Struct #679

Merged
merged 7 commits into from
Mar 1, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/dom/Null.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export class Null extends Value(Object, IonTypes.NULL, FromJsConstructor.NONE) {
this._unsupportedOperationOrNullDereference("fieldNames");
}

fields(): [string, Value][] {
fields(): [string, Value[]][] {
desaikd marked this conversation as resolved.
Show resolved Hide resolved
this._unsupportedOperationOrNullDereference("fields");
}

Expand Down
55 changes: 40 additions & 15 deletions src/dom/Struct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export class Struct extends Value(
* @param fields An iterator of field name/value pairs to represent as a struct.
* @param annotations An optional array of strings to associate with this null value.
*/
constructor(fields: Iterable<[string, Value]>, annotations: string[] = []) {
constructor(fields: Iterable<[string, Value[]]>, annotations: string[] = []) {
super();
for (const [fieldName, fieldValue] of fields) {
this._fields[fieldName] = fieldValue;
Expand All @@ -53,7 +53,7 @@ export class Struct extends Value(
// All values set by the user are stored in `this._fields` to avoid
// potentially overwriting Struct methods.
set: function (target, name, value): boolean {
target._fields[name] = value;
target._fields[name] = [value];
return true; // Indicates that the assignment succeeded
},
get: function (target, name): any {
Expand All @@ -62,7 +62,12 @@ export class Struct extends Value(
if (name in target) {
return target[name];
}
return target._fields[name];
let length =
target._fields[name] !== undefined ? target._fields[name].length : -1;
if (length === -1) {
return target._fields[name];
}
return target._fields[name][length - 1];
},
deleteProperty: function (target, name): boolean {
// Property is deleted only if it's in _field Collection
Expand All @@ -83,29 +88,29 @@ export class Struct extends Value(
if (typeof pathHead !== "string") {
throw new Error(`Cannot index into a struct with a ${typeof pathHead}.`);
}
const child: Value | undefined = this._fields[pathHead];
const child: Value[] | undefined = this._fields[pathHead];
if (child === undefined) {
return null;
}
if (pathTail.length === 0) {
return child;
return child[child.length - 1]!;
}
return child.get(...pathTail);
return child[child.length - 1]!.get(...pathTail);
}

fieldNames(): string[] {
return Object.keys(this._fields);
}

fields(): [string, Value][] {
fields(): [string, Value[]][] {
return Object.entries(this._fields);
}

elements(): Value[] {
return Object.values(this._fields);
}

[Symbol.iterator](): IterableIterator<[string, Value]> {
[Symbol.iterator](): IterableIterator<[string, Value[]]> {
return this.fields()[Symbol.iterator]();
}

Expand All @@ -122,9 +127,11 @@ export class Struct extends Value(
writeTo(writer: Writer): void {
writer.setAnnotations(this.getAnnotations());
writer.stepIn(IonTypes.STRUCT);
for (const [fieldName, value] of this) {
for (const [fieldName, values] of this) {
writer.writeFieldName(fieldName);
value.writeTo(writer);
for (let value of values) {
value.writeTo(writer);
}
}
writer.stepOut();
}
Expand All @@ -138,16 +145,20 @@ export class Struct extends Value(
}

toJSON() {
return this._fields;
let normalizedFields = Object.create(null);
for (const [key, values] of this) {
normalizedFields[key] = values[values.length - 1];
}
return normalizedFields;
}

static _fromJsValue(jsValue: any, annotations: string[]): Value {
if (!(jsValue instanceof Object)) {
throw new Error(`Cannot create a dom.Struct from: ${jsValue.toString()}`);
}
const fields: [string, Value][] = Object.entries(
const fields: [string, Value[]][] = Object.entries(
jsValue
).map(([key, value]) => [key, Value.from(value)]);
).map(([key, value]) => [key, [Value.from(value)]]);
return new this(fields, annotations);
}

Expand Down Expand Up @@ -177,7 +188,7 @@ export class Struct extends Value(
// We will consider other Struct-ish types
if (typeof other === "object" || other instanceof global.Object) {
isSupportedType = true;
valueToCompare = Object.entries(other);
valueToCompare = Value.from(other).fields();
}
}

Expand All @@ -198,7 +209,7 @@ export class Struct extends Value(
const expectedChild = valueToCompare[j];
matchFound =
child[0] === expectedChild[0] &&
child[1].equals(expectedChild[1], options);
this._ionValueEquals(child[1], expectedChild[1], options);
if (matchFound) {
paired[j] = true;
}
Expand All @@ -214,4 +225,18 @@ export class Struct extends Value(
}
return matchFound;
}

// helper function for comparison of all values of a field
_ionValueEquals(child: Value[], expectedChild: Value[], options): boolean {
if (child.length !== expectedChild.length) {
return false;
}

for (let i: number = 0; i < child.length; i++) {
if (!child[i].equals(expectedChild[i], options)) {
return false;
}
}
return true;
}
}
4 changes: 2 additions & 2 deletions src/dom/Value.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export interface Value {
* For the Struct type, returns an array containing the field name/value pairs in the Struct;
* otherwise throws an Error.
*/
fields(): [string, Value][];
fields(): [string, Value[]][];

/**
* For the Struct, List, and SExpression types, returns an array containing the container's
Expand Down Expand Up @@ -316,7 +316,7 @@ export function Value<Clazz extends Constructor>(
this._unsupportedOperation("fieldNames");
}

fields(): [string, Value][] {
fields(): [string, Value[]][] {
this._unsupportedOperation("fields");
}

Expand Down
8 changes: 6 additions & 2 deletions src/dom/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,15 @@ function _loadValue(reader: Reader): Value {
}

function _loadStruct(reader: Reader): Struct {
const children: Map<string, Value> = new Map();
const children: Map<string, Value[]> = new Map();
const annotations: string[] = reader.annotations();
reader.stepIn();
while (reader.next()) {
children.set(reader.fieldName()!, _loadValue(reader));
if (children.has(reader.fieldName()!)) {
children.get(reader.fieldName()!)!.push(_loadValue(reader));
} else {
children.set(reader.fieldName()!, [_loadValue(reader)]);
}
}
reader.stepOut();
return new Struct(children.entries(), annotations);
Expand Down
33 changes: 29 additions & 4 deletions test/dom/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -455,10 +455,10 @@ describe('DOM', () => {
assert.equal("Jacob", s.get("name", "middle")!.stringValue());

// Iteration
for (let [fieldName, value] of s.fields()) {
for (let [fieldName, values] of s.fields()) {
assert.isTrue(typeof fieldName === "string");
assert.isTrue(fieldName.length > 0);
assert.isFalse(value.isNull());
values.forEach(value => {assert.isFalse(value.isNull())});
}

assert.equal(2, s.fields().length);
Expand All @@ -484,12 +484,37 @@ describe('DOM', () => {
assert.equal("Jacob", s["name"]["middle"]);

// Iteration
for (let [fieldName, value] of s) {
for (let [fieldName, values] of s) {
assert.isTrue(typeof fieldName === "string");
assert.isTrue(fieldName.length > 0);
assert.isFalse(value.isNull());
values.forEach(value => assert.isFalse(value.isNull()));
}

assert.equal(2, s.fields().length)
});

it('load() Struct with duplicate fields as any', () => {
let s: any = load(
'foo::bar::{' +
'age: 55, ' +
'age: 41' +
'}'
)!;

assert.equal(IonTypes.STRUCT, s.getType());
assert.deepEqual(['foo', 'bar'], s.getAnnotations());

// Field access return last value for a fieldname
assert.equal(41, s.age);
desaikd marked this conversation as resolved.
Show resolved Hide resolved

// Iteration
for (let [fieldName, values] of s) {
assert.isTrue(typeof fieldName === "string");
assert.isTrue(fieldName.length > 0);
values.forEach(value => assert.isFalse(value.isNull()));
}

// length is 1 with two values: 55, 41
assert.equal(1, s.fields().length)
});
});
9 changes: 9 additions & 0 deletions test/dom/equivalence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -347,4 +347,13 @@ describe("Equivalence", () => {
)
);
});
it("equals() for Struct with duplicate fields", () => {
let value1: Value = load("{ foo: 'bar', foo: 'baz' }")!;
let value2: Value = load("{ foo: 'bar', foo: 'baz' }")!;
let value3: Value = load("{ foo: 'bar', foo: 'qux' }")!;
desaikd marked this conversation as resolved.
Show resolved Hide resolved

assert.isTrue(value1.equals(value2));
assert.isTrue(value1.equals(value2));
desaikd marked this conversation as resolved.
Show resolved Hide resolved
assert.isFalse(value1.equals(value3));
});
});