Skip to content

Commit

Permalink
fix: fix codegen for maps with wrapper value type (#370)
Browse files Browse the repository at this point in the history
  • Loading branch information
@aikoven
aikoven committed Oct 28, 2021
1 parent 3a0f2cb commit dd2481d
Show file tree
Hide file tree
Showing 7 changed files with 177 additions and 7 deletions.
11 changes: 10 additions & 1 deletion integration/simple/google/protobuf/timestamp.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,16 @@ export const protobufPackage = 'google.protobuf';
* .setNanos((int) ((millis % 1000) * 1000000)).build();
*
*
* Example 5: Compute Timestamp from current time in Python.
* Example 5: Compute Timestamp from Java `Instant.now()`.
*
* Instant now = Instant.now();
*
* Timestamp timestamp =
* Timestamp.newBuilder().setSeconds(now.getEpochSecond())
* .setNanos(now.getNano()).build();
*
*
* Example 6: Compute Timestamp from current time in Python.
*
* timestamp = Timestamp()
* timestamp.GetCurrentTime()
Expand Down
5 changes: 5 additions & 0 deletions integration/simple/simple-json-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ describe('simple json', () => {
"1": 0,
},
"mapOfBytes": Object {},
"mapOfStringValues": Object {},
"mapOfTimestamps": Object {},
"nameLookup": Object {},
}
Expand All @@ -212,6 +213,7 @@ describe('simple json', () => {
"entitiesById": Object {},
"intLookup": Object {},
"mapOfBytes": Object {},
"mapOfStringValues": Object {},
"mapOfTimestamps": Object {
"a": 1970-01-01T00:16:40.000Z,
"b": 1970-01-01T00:33:20.000Z,
Expand All @@ -233,6 +235,7 @@ describe('simple json', () => {
a: new Uint8Array([1, 2]),
b: new Uint8Array([1, 2, 3]),
},
mapOfStringValues: {},
};
const json = SimpleWithMap.toJSON(s1);
expect(json).toMatchInlineSnapshot(`
Expand All @@ -243,6 +246,7 @@ describe('simple json', () => {
"a": "AQI=",
"b": "AQID",
},
"mapOfStringValues": Object {},
"mapOfTimestamps": Object {},
"nameLookup": Object {},
}
Expand Down Expand Up @@ -272,6 +276,7 @@ describe('simple json', () => {
3,
],
},
"mapOfStringValues": Object {},
"mapOfTimestamps": Object {},
"nameLookup": Object {},
}
Expand Down
40 changes: 36 additions & 4 deletions integration/simple/simple-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,11 +231,34 @@ describe('simple', () => {
intLookup: { 1: 2, 2: 1 },
mapOfTimestamps: {},
mapOfBytes: {},
mapOfStringValues: { a: '1', b: '', c: undefined },
};
const s2 = PbSimpleWithMap.toObject(PbSimpleWithMap.decode(SimpleWithMap.encode(s1).finish()));
delete (s1 as any).mapOfTimestamps;
delete (s1 as any).mapOfBytes;
expect(s2).toEqual(s1);
expect(s2).toMatchInlineSnapshot(`
Object {
"entitiesById": Object {
"1": Object {
"id": 1,
},
"2": Object {
"id": 2,
},
},
"intLookup": Object {
"1": 2,
"2": 1,
},
"mapOfStringValues": Object {
"a": Object {
"value": "1",
},
"b": Object {},
},
"nameLookup": Object {
"foo": "bar",
},
}
`);
});

it('can encode maps with default values', () => {
Expand All @@ -245,6 +268,7 @@ describe('simple', () => {
intLookup: { 1: 0 },
mapOfTimestamps: {},
mapOfBytes: {},
mapOfStringValues: { foo: '', bar: undefined },
};
const s2 = SimpleWithMap.decode(SimpleWithMap.encode(s1).finish());
expect(s2).toEqual(s1);
Expand All @@ -264,6 +288,7 @@ describe('simple', () => {
nameLookup: {},
mapOfTimestamps: {},
mapOfBytes: {},
mapOfStringValues: {},
});
});

Expand All @@ -289,7 +314,10 @@ describe('simple', () => {
});

it('can fromPartial on maps with falsey values', () => {
const s1 = SimpleWithMap.fromPartial({ intLookup: { 1: 2, 2: 0 } });
const s1 = SimpleWithMap.fromPartial({
intLookup: { 1: 2, 2: 0 },
mapOfStringValues: { a: '1', b: '', c: undefined },
});
expect(s1).toMatchInlineSnapshot(`
Object {
"entitiesById": Object {},
Expand All @@ -298,6 +326,10 @@ describe('simple', () => {
"2": 0,
},
"mapOfBytes": Object {},
"mapOfStringValues": Object {
"a": "1",
"b": "",
},
"mapOfTimestamps": Object {},
"nameLookup": Object {},
}
Expand Down
Binary file modified integration/simple/simple.bin
View file
Binary file not shown.
1 change: 1 addition & 0 deletions integration/simple/simple.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ message SimpleWithMap {
map<int32, int32> intLookup = 3;
map<string, google.protobuf.Timestamp> mapOfTimestamps = 4;
map<string, bytes> mapOfBytes = 5;
map<string, google.protobuf.StringValue> mapOfStringValues = 6;
}

message SimpleWithSnakeCaseMap {
Expand Down
110 changes: 110 additions & 0 deletions integration/simple/simple.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ export interface SimpleWithMap {
intLookup: { [key: number]: number };
mapOfTimestamps: { [key: string]: Date };
mapOfBytes: { [key: string]: Uint8Array };
mapOfStringValues: { [key: string]: string | undefined };
}

export interface SimpleWithMap_EntitiesByIdEntry {
Expand Down Expand Up @@ -220,6 +221,11 @@ export interface SimpleWithMap_MapOfBytesEntry {
value: Uint8Array;
}

export interface SimpleWithMap_MapOfStringValuesEntry {
key: string;
value: string | undefined;
}

export interface SimpleWithSnakeCaseMap {
entitiesById: { [key: number]: Entity };
}
Expand Down Expand Up @@ -1182,6 +1188,11 @@ export const SimpleWithMap = {
Object.entries(message.mapOfBytes).forEach(([key, value]) => {
SimpleWithMap_MapOfBytesEntry.encode({ key: key as any, value }, writer.uint32(42).fork()).ldelim();
});
Object.entries(message.mapOfStringValues).forEach(([key, value]) => {
if (value !== undefined) {
SimpleWithMap_MapOfStringValuesEntry.encode({ key: key as any, value }, writer.uint32(50).fork()).ldelim();
}
});
return writer;
},

Expand All @@ -1194,6 +1205,7 @@ export const SimpleWithMap = {
message.intLookup = {};
message.mapOfTimestamps = {};
message.mapOfBytes = {};
message.mapOfStringValues = {};
while (reader.pos < end) {
const tag = reader.uint32();
switch (tag >>> 3) {
Expand Down Expand Up @@ -1227,6 +1239,12 @@ export const SimpleWithMap = {
message.mapOfBytes[entry5.key] = entry5.value;
}
break;
case 6:
const entry6 = SimpleWithMap_MapOfStringValuesEntry.decode(reader, reader.uint32());
if (entry6.value !== undefined) {
message.mapOfStringValues[entry6.key] = entry6.value;
}
break;
default:
reader.skipType(tag & 7);
break;
Expand All @@ -1242,6 +1260,7 @@ export const SimpleWithMap = {
message.intLookup = {};
message.mapOfTimestamps = {};
message.mapOfBytes = {};
message.mapOfStringValues = {};
if (object.entitiesById !== undefined && object.entitiesById !== null) {
Object.entries(object.entitiesById).forEach(([key, value]) => {
message.entitiesById[Number(key)] = Entity.fromJSON(value);
Expand All @@ -1267,6 +1286,11 @@ export const SimpleWithMap = {
message.mapOfBytes[key] = bytesFromBase64(value as string);
});
}
if (object.mapOfStringValues !== undefined && object.mapOfStringValues !== null) {
Object.entries(object.mapOfStringValues).forEach(([key, value]) => {
message.mapOfStringValues[key] = value as string | undefined;
});
}
return message;
},

Expand Down Expand Up @@ -1302,6 +1326,12 @@ export const SimpleWithMap = {
obj.mapOfBytes[k] = base64FromBytes(v);
});
}
obj.mapOfStringValues = {};
if (message.mapOfStringValues) {
Object.entries(message.mapOfStringValues).forEach(([k, v]) => {
obj.mapOfStringValues[k] = v;
});
}
return obj;
},

Expand All @@ -1312,6 +1342,7 @@ export const SimpleWithMap = {
message.intLookup = {};
message.mapOfTimestamps = {};
message.mapOfBytes = {};
message.mapOfStringValues = {};
if (object.entitiesById !== undefined && object.entitiesById !== null) {
Object.entries(object.entitiesById).forEach(([key, value]) => {
if (value !== undefined) {
Expand Down Expand Up @@ -1347,6 +1378,13 @@ export const SimpleWithMap = {
}
});
}
if (object.mapOfStringValues !== undefined && object.mapOfStringValues !== null) {
Object.entries(object.mapOfStringValues).forEach(([key, value]) => {
if (value !== undefined) {
message.mapOfStringValues[key] = value;
}
});
}
return message;
},
};
Expand Down Expand Up @@ -1712,6 +1750,78 @@ export const SimpleWithMap_MapOfBytesEntry = {
},
};

const baseSimpleWithMap_MapOfStringValuesEntry: object = { key: '' };

export const SimpleWithMap_MapOfStringValuesEntry = {
encode(message: SimpleWithMap_MapOfStringValuesEntry, writer: Writer = Writer.create()): Writer {
if (message.key !== '') {
writer.uint32(10).string(message.key);
}
if (message.value !== undefined) {
StringValue.encode({ value: message.value! }, writer.uint32(18).fork()).ldelim();
}
return writer;
},

decode(input: Reader | Uint8Array, length?: number): SimpleWithMap_MapOfStringValuesEntry {
const reader = input instanceof Reader ? input : new Reader(input);
let end = length === undefined ? reader.len : reader.pos + length;
const message = { ...baseSimpleWithMap_MapOfStringValuesEntry } as SimpleWithMap_MapOfStringValuesEntry;
while (reader.pos < end) {
const tag = reader.uint32();
switch (tag >>> 3) {
case 1:
message.key = reader.string();
break;
case 2:
message.value = StringValue.decode(reader, reader.uint32()).value;
break;
default:
reader.skipType(tag & 7);
break;
}
}
return message;
},

fromJSON(object: any): SimpleWithMap_MapOfStringValuesEntry {
const message = { ...baseSimpleWithMap_MapOfStringValuesEntry } as SimpleWithMap_MapOfStringValuesEntry;
if (object.key !== undefined && object.key !== null) {
message.key = String(object.key);
} else {
message.key = '';
}
if (object.value !== undefined && object.value !== null) {
message.value = String(object.value);
} else {
message.value = undefined;
}
return message;
},

toJSON(message: SimpleWithMap_MapOfStringValuesEntry): unknown {
const obj: any = {};
message.key !== undefined && (obj.key = message.key);
message.value !== undefined && (obj.value = message.value);
return obj;
},

fromPartial(object: DeepPartial<SimpleWithMap_MapOfStringValuesEntry>): SimpleWithMap_MapOfStringValuesEntry {
const message = { ...baseSimpleWithMap_MapOfStringValuesEntry } as SimpleWithMap_MapOfStringValuesEntry;
if (object.key !== undefined && object.key !== null) {
message.key = object.key;
} else {
message.key = '';
}
if (object.value !== undefined && object.value !== null) {
message.value = object.value;
} else {
message.value = undefined;
}
return message;
},
};

const baseSimpleWithSnakeCaseMap: object = {};

export const SimpleWithSnakeCaseMap = {
Expand Down
17 changes: 15 additions & 2 deletions src/main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -798,10 +798,18 @@ function generateEncode(ctx: Context, fullName: string, messageDesc: DescriptorP

if (isRepeated(field)) {
if (isMapType(ctx, messageDesc, field)) {
const valueType = (typeMap.get(field.typeName)![2] as DescriptorProto).field[1];
const maybeTypeField = options.outputTypeRegistry ? `$type: '${field.typeName.slice(1)}',` : '';
const entryWriteSnippet = isValueType(ctx, valueType)
? code`
if (value !== undefined) {
${writeSnippet(`{ ${maybeTypeField} key: key as any, value }`)};
}
`
: writeSnippet(`{ ${maybeTypeField} key: key as any, value }`);
chunks.push(code`
Object.entries(message.${fieldName}).forEach(([key, value]) => {
${writeSnippet(`{ ${maybeTypeField} key: key as any, value }`)};
${entryWriteSnippet}
});
`);
} else if (packedType(field.type) === undefined) {
Expand Down Expand Up @@ -967,6 +975,9 @@ function generateFromJson(ctx: Context, fullName: string, messageDesc: Descripto
(options.useDate === DateOption.DATE || options.useDate === DateOption.TIMESTAMP)
) {
return code`${utils.fromJsonTimestamp}(${from})`;
} else if (isValueType(ctx, valueType)) {
const type = basicTypeName(ctx, valueType);
return code`${from} as ${type}`;
} else {
const type = basicTypeName(ctx, valueType);
return code`${type}.fromJSON(${from})`;
Expand Down Expand Up @@ -1064,7 +1075,7 @@ function generateToJson(ctx: Context, fullName: string, messageDesc: DescriptorP
return code`${from}`;
} else if (isTimestamp(valueType) && options.useDate === DateOption.TIMESTAMP) {
return code`${utils.fromTimestamp}(${from}).toISOString()`;
} else if (isScalar(valueType)) {
} else if (isScalar(valueType) || isValueType(ctx, valueType)) {
return code`${from}`;
} else {
const type = basicTypeName(ctx, valueType);
Expand Down Expand Up @@ -1168,6 +1179,8 @@ function generateFromPartial(ctx: Context, fullName: string, messageDesc: Descri
(options.useDate === DateOption.DATE || options.useDate === DateOption.STRING)
) {
return code`${from}`;
} else if (isValueType(ctx, valueType)) {
return code`${from}`;
} else {
const type = basicTypeName(ctx, valueType);
return code`${type}.fromPartial(${from})`;
Expand Down

0 comments on commit dd2481d

Please sign in to comment.