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

fix: Add missing defaults to fromPartial if options.oneof is UNIONS #375

Merged
merged 3 commits into from
Oct 26, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
25 changes: 22 additions & 3 deletions integration/oneof-unions/oneof-unions-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,22 @@ import { PleaseChoose } from './oneof';

describe('oneof=unions', () => {
it('generates types correctly', () => {
const alice: PleaseChoose = { name: 'Alice', age: 42 };
const bob: PleaseChoose = { name: 'Bob', age: 42, choice: { $case: 'aNumber', aNumber: 132 } };
const alice: PleaseChoose = {
name: 'Alice',
age: 42,
signature: new Uint8Array([0xab, 0xcd]),
};
const bob: PleaseChoose = {
name: 'Bob',
age: 42,
choice: { $case: 'aNumber', aNumber: 132 },
signature: new Uint8Array([0xab, 0xcd]),
};
const charlie: PleaseChoose = {
name: 'Charlie',
age: 42,
choice: { $case: 'aMessage', aMessage: { name: 'charlie' } },
signature: new Uint8Array([0xab, 0xcd]),
};
});

Expand All @@ -27,6 +37,7 @@ describe('oneof=unions', () => {
age: 37,
choice: { $case: 'aBool', aBool: true },
eitherOr: { $case: 'or', or: 'perhaps not' },
signature: new Uint8Array(),
});
});

Expand All @@ -36,13 +47,15 @@ describe('oneof=unions', () => {
age: 37,
choice: { $case: 'aBool', aBool: true },
eitherOr: { $case: 'or', or: 'perhaps not' },
signature: new Uint8Array([0xab, 0xcd]),
}).finish();
let decoded = pbjs.PleaseChoose.decode(encoded);
expect(decoded).toEqual({
name: 'Debbie',
aBool: true,
age: 37,
or: 'perhaps not',
signature: Buffer.from([0xab, 0xcd]),
});
});

Expand All @@ -51,19 +64,22 @@ describe('oneof=unions', () => {
expect(empty).toEqual({
name: '',
age: 0,
signature: new Uint8Array(),
});

let partial = PleaseChoose.fromPartial({
name: 'Debbie',
age: 37,
choice: { $case: 'aBool', aBool: true },
eitherOr: { $case: 'or', or: 'perhaps not' },
signature: new Uint8Array([0xab, 0xcd]),
});
expect(partial).toEqual({
name: 'Debbie',
age: 37,
choice: { $case: 'aBool', aBool: true },
eitherOr: { $case: 'or', or: 'perhaps not' },
signature: new Uint8Array([0xab, 0xcd]),
});
});

Expand All @@ -73,6 +89,7 @@ describe('oneof=unions', () => {
age: 37,
choice: { $case: 'aBool', aBool: true },
eitherOr: { $case: 'or', or: 'perhaps not' },
signature: new Uint8Array([0xab, 0xcd]),
};
let pbjsJson = pbjs.PleaseChoose.decode(PleaseChoose.encode(debbie).finish()).toJSON();
let json = PleaseChoose.toJSON(debbie);
Expand All @@ -81,13 +98,14 @@ describe('oneof=unions', () => {

it('fromJSON', () => {
let empty = PleaseChoose.fromJSON({});
expect(empty).toEqual({ age: 0, name: '' });
expect(empty).toEqual({ age: 0, name: '', signature: new Uint8Array() });

let debbie: PleaseChoose = {
name: 'Debbie',
age: 37,
choice: { $case: 'aBool', aBool: true },
eitherOr: { $case: 'or', or: 'perhaps not' },
signature: new Uint8Array([0xab, 0xcd]),
};
let pbjsJson = pbjs.PleaseChoose.decode(PleaseChoose.encode(debbie).finish()).toJSON();
let fromJson = PleaseChoose.fromJSON(pbjsJson);
Expand All @@ -99,6 +117,7 @@ describe('oneof=unions', () => {
name: 'Debbie',
age: 37,
choice: { $case: 'aNumber', aNumber: 42 },
signature: Buffer.from([0xab, 0xcd]),
};
let encoded = PleaseChoose.encode(obj).finish();
let decoded = PleaseChoose.decode(encoded);
Expand Down
Binary file modified integration/oneof-unions/oneof.bin
Binary file not shown.
1 change: 1 addition & 0 deletions integration/oneof-unions/oneof.proto
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ message PleaseChoose {
string third_option = 9;
}

bytes signature = 12;
}

/** For testing proto3's field presence feature. */
Expand Down
29 changes: 29 additions & 0 deletions integration/oneof-unions/oneof.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export interface PleaseChoose {
| { $case: 'either'; either: string }
| { $case: 'or'; or: string }
| { $case: 'thirdOption'; thirdOption: string };
signature: Uint8Array;
}

export enum PleaseChoose_StateEnum {
Expand Down Expand Up @@ -105,13 +106,17 @@ export const PleaseChoose = {
if (message.eitherOr?.$case === 'thirdOption') {
writer.uint32(74).string(message.eitherOr.thirdOption);
}
if (message.signature.length !== 0) {
writer.uint32(98).bytes(message.signature);
}
return writer;
},

decode(input: Reader | Uint8Array, length?: number): PleaseChoose {
const reader = input instanceof Reader ? input : new Reader(input);
let end = length === undefined ? reader.len : reader.pos + length;
const message = { ...basePleaseChoose } as PleaseChoose;
message.signature = new Uint8Array();
while (reader.pos < end) {
const tag = reader.uint32();
switch (tag >>> 3) {
Expand Down Expand Up @@ -148,6 +153,9 @@ export const PleaseChoose = {
case 9:
message.eitherOr = { $case: 'thirdOption', thirdOption: reader.string() };
break;
case 12:
message.signature = reader.bytes();
break;
default:
reader.skipType(tag & 7);
break;
Expand All @@ -158,6 +166,7 @@ export const PleaseChoose = {

fromJSON(object: any): PleaseChoose {
const message = { ...basePleaseChoose } as PleaseChoose;
message.signature = new Uint8Array();
if (object.name !== undefined && object.name !== null) {
message.name = String(object.name);
}
Expand Down Expand Up @@ -191,6 +200,9 @@ export const PleaseChoose = {
if (object.thirdOption !== undefined && object.thirdOption !== null) {
message.eitherOr = { $case: 'thirdOption', thirdOption: String(object.thirdOption) };
}
if (object.signature !== undefined && object.signature !== null) {
message.signature = bytesFromBase64(object.signature);
}
return message;
},

Expand All @@ -212,13 +224,17 @@ export const PleaseChoose = {
message.eitherOr?.$case === 'either' && (obj.either = message.eitherOr?.either);
message.eitherOr?.$case === 'or' && (obj.or = message.eitherOr?.or);
message.eitherOr?.$case === 'thirdOption' && (obj.thirdOption = message.eitherOr?.thirdOption);
message.signature !== undefined &&
(obj.signature = base64FromBytes(message.signature !== undefined ? message.signature : new Uint8Array()));
return obj;
},

fromPartial(object: DeepPartial<PleaseChoose>): PleaseChoose {
const message = { ...basePleaseChoose } as PleaseChoose;
if (object.name !== undefined && object.name !== null) {
message.name = object.name;
} else {
message.name = '';
}
if (object.choice?.$case === 'aNumber' && object.choice?.aNumber !== undefined && object.choice?.aNumber !== null) {
message.choice = { $case: 'aNumber', aNumber: object.choice.aNumber };
Expand Down Expand Up @@ -248,6 +264,8 @@ export const PleaseChoose = {
}
if (object.age !== undefined && object.age !== null) {
message.age = object.age;
} else {
message.age = 0;
Copy link
Collaborator Author

@webmaster128 webmaster128 Oct 25, 2021

Choose a reason for hiding this comment

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

Same code as in oneof-properties now

}
if (
object.eitherOr?.$case === 'either' &&
Expand All @@ -266,6 +284,11 @@ export const PleaseChoose = {
) {
message.eitherOr = { $case: 'thirdOption', thirdOption: object.eitherOr.thirdOption };
}
if (object.signature !== undefined && object.signature !== null) {
message.signature = object.signature;
} else {
message.signature = new Uint8Array();
}
Comment on lines +289 to +291
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Before this change, those 3 lines were missing.

return message;
},
};
Expand Down Expand Up @@ -316,6 +339,8 @@ export const PleaseChoose_Submessage = {
const message = { ...basePleaseChoose_Submessage } as PleaseChoose_Submessage;
if (object.name !== undefined && object.name !== null) {
message.name = object.name;
} else {
message.name = '';
}
return message;
},
Expand Down Expand Up @@ -377,9 +402,13 @@ export const SimpleButOptional = {
const message = { ...baseSimpleButOptional } as SimpleButOptional;
if (object.name !== undefined && object.name !== null) {
message.name = object.name;
} else {
message.name = undefined;
}
if (object.age !== undefined && object.age !== null) {
message.age = object.age;
} else {
message.age = undefined;
}
return message;
},
Expand Down
2 changes: 1 addition & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1222,7 +1222,7 @@ function generateFromPartial(ctx: Context, fullName: string, messageDesc: Descri
}
}

if (!isRepeated(field) && options.oneof !== OneofOption.UNIONS) {
if (!isRepeated(field) && !isWithinOneOfThatShouldBeUnion(options, field)) {
chunks.push(code`} else {`);
const v = isWithinOneOf(field) ? 'undefined' : defaultValue(ctx, field);
chunks.push(code`message.${fieldName} = ${v}`);
Expand Down