Merge pull request #255 from proto-graphql/izumin5210/proto3-optional

izumin5210 · web-flow · commit 3f531e44f112 · 2022-12-24T11:52:27.000+09:00
feat: support proto3 optional
diff --git a/.changeset/weak-apes-behave.md b/.changeset/weak-apes-behave.md
@@ -0,0 +1,10 @@
+---
+"@proto-graphql/e2e": patch
+"@proto-graphql/codegen-core": patch
+"@proto-graphql/proto-descriptors": patch
+"@testapis/proto": patch
+"protoc-gen-nexus": patch
+"protoc-gen-pothos": patch
+---
+
+support proto3 optional
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -15,6 +15,8 @@ jobs:
         node-version: 18.x
     - run: yarn --frozen-lockfile
     - run: yarn test
+      env:
+        NODE_OPTIONS: "--max_old_space_size=8192"
     - name: Coveralls
       uses: coverallsapp/github-action@v1.1.2
       with:
@@ -38,3 +40,5 @@ jobs:
     - uses: bufbuild/buf-setup-action@v1
     - run: yarn --frozen-lockfile
     - run: yarn test:e2e
+      env:
+        NODE_OPTIONS: "--max_old_space_size=8192"
diff --git a/e2e/tests.json b/e2e/tests.json
@@ -22,6 +22,8 @@
     { "target": "nexus", "proto": { "package": "oneof", "lib": "protobufjs" } },
     { "target": "nexus", "proto": { "package": "primitives", "lib": "google-protobuf" } },
     { "target": "nexus", "proto": { "package": "primitives", "lib": "protobufjs" } },
+    { "target": "nexus", "proto": { "package": "proto3_optional", "lib": "google-protobuf" } },
+    { "target": "nexus", "proto": { "package": "proto3_optional", "lib": "protobufjs" } },
     { "target": "nexus", "proto": { "package": "wktypes", "lib": "google-protobuf" } },
     { "target": "nexus", "proto": { "package": "wktypes", "lib": "protobufjs" } },
     { "target": "pothos", "proto": { "package": "deprecation", "lib": "ts-proto" } },
@@ -36,6 +38,7 @@
     { "target": "pothos", "proto": { "package": "oneof", "lib": "ts-proto" } },
     { "target": "pothos", "proto": { "package": "primitives", "lib": "ts-proto" } },
     { "target": "pothos", "proto": { "package": "primitives", "lib": "ts-proto-with-forcelong-number" } },
+    { "target": "pothos", "proto": { "package": "proto3_optional", "lib": "ts-proto" } },
     { "target": "pothos", "proto": { "package": "wktypes", "lib": "ts-proto" } },
     { "target": "pothos", "proto": { "package": "wktypes", "lib": "ts-proto-with-forcelong-number" } }
   ]
diff --git a/e2e/tests/nexus--proto3_optional--google-protobuf/__generated__/schema.graphql b/e2e/tests/nexus--proto3_optional--google-protobuf/__generated__/schema.graphql
diff --git a/e2e/tests/nexus--proto3_optional--google-protobuf/schema.ts b/e2e/tests/nexus--proto3_optional--google-protobuf/schema.ts
@@ -0,0 +1,18 @@
+import "@proto-nexus/google-protobuf";
+import { Message } from "@testapis/node-native/lib/testapis/proto3_optional/proto3_optional_pb";
+import { queryField } from "nexus";
+
+import { makeTestSchema } from "../../src/makeTestSchema";
+import * as types1 from "../__generated__/nexus/google-protobuf/testapis/proto3_optional/proto3_optional_pb_nexus";
+
+const testQuery = queryField("test1", {
+  type: "Message",
+  resolve() {
+    return new Message();
+  },
+});
+
+export const schema = makeTestSchema({
+  rootDir: __dirname,
+  types: [types1, testQuery],
+});
diff --git a/e2e/tests/nexus--proto3_optional--google-protobuf/tsconfig.json b/e2e/tests/nexus--proto3_optional--google-protobuf/tsconfig.json
@@ -0,0 +1,17 @@
+// Code generated by setupTests.mjs. DO NOT EDIT.
+{
+  "extends": "../../tsconfig.base.json",
+  "compilerOptions": {
+    "baseUrl": ".",
+    "paths": {
+      "@testapp/*": [
+        "./*"
+      ]
+    }
+  },
+  "include": [
+    "./**/*",
+    "../../src/**/*",
+    "../__generated__/nexus/google-protobuf/testapis/proto3_optional/**/*"
+  ]
+}
diff --git a/e2e/tests/nexus--proto3_optional--protobufjs/__generated__/schema.graphql b/e2e/tests/nexus--proto3_optional--protobufjs/__generated__/schema.graphql
diff --git a/e2e/tests/nexus--proto3_optional--protobufjs/schema.ts b/e2e/tests/nexus--proto3_optional--protobufjs/schema.ts
@@ -0,0 +1,18 @@
+import "@proto-nexus/google-protobuf";
+import * as pb from "@testapis/node/lib/testapis/proto3_optional";
+import { queryField } from "nexus";
+
+import { makeTestSchema } from "../../src/makeTestSchema";
+import * as types1 from "../__generated__/nexus/protobufjs/testapis/proto3_optional/proto3_optional_pb_nexus";
+
+const testQuery = queryField("test1", {
+  type: "Message",
+  resolve() {
+    return new pb.testapis.proto3_optional.Message();
+  },
+});
+
+export const schema = makeTestSchema({
+  rootDir: __dirname,
+  types: [types1, testQuery],
+});
diff --git a/e2e/tests/nexus--proto3_optional--protobufjs/tsconfig.json b/e2e/tests/nexus--proto3_optional--protobufjs/tsconfig.json
@@ -0,0 +1,17 @@
+// Code generated by setupTests.mjs. DO NOT EDIT.
+{
+  "extends": "../../tsconfig.base.json",
+  "compilerOptions": {
+    "baseUrl": ".",
+    "paths": {
+      "@testapp/*": [
+        "./*"
+      ]
+    }
+  },
+  "include": [
+    "./**/*",
+    "../../src/**/*",
+    "../__generated__/nexus/protobufjs/testapis/proto3_optional/**/*"
+  ]
+}
diff --git a/e2e/tests/pothos--proto3_optional--ts-proto/__generated__/schema.graphql b/e2e/tests/pothos--proto3_optional--ts-proto/__generated__/schema.graphql
diff --git a/e2e/tests/pothos--proto3_optional--ts-proto/builder.ts b/e2e/tests/pothos--proto3_optional--ts-proto/builder.ts
@@ -0,0 +1,4 @@
+import SchemaBuilder from "@pothos/core";
+
+export const builder = new SchemaBuilder({});
+builder.queryType({});
diff --git a/e2e/tests/pothos--proto3_optional--ts-proto/schema.test.ts b/e2e/tests/pothos--proto3_optional--ts-proto/schema.test.ts
@@ -0,0 +1,57 @@
+import { graphql } from "graphql";
+
+import { schema } from "./schema";
+
+it("processes a query successfully", async () => {
+  const resp = await graphql({
+    schema,
+    source: /* GraphQL */ `
+      query Test {
+        valuesArePresent {
+          ...Message
+        }
+      }
+      fragment Message on Message {
+        requiredStringValue
+        optionalStringValue
+      }
+    `,
+  });
+  expect(resp).toMatchInlineSnapshot(`
+    {
+      "data": {
+        "valuesArePresent": {
+          "optionalStringValue": "optional field",
+          "requiredStringValue": "required field",
+        },
+      },
+    }
+  `);
+});
+
+it("processes a query successfully when values are blank", async () => {
+  const resp = await graphql({
+    schema,
+    source: /* GraphQL */ `
+      query Test {
+        valuesAreBlank {
+          ...Message
+        }
+      }
+      fragment Message on Message {
+        requiredStringValue
+        optionalStringValue
+      }
+    `,
+  });
+  expect(resp).toMatchInlineSnapshot(`
+    {
+      "data": {
+        "valuesAreBlank": {
+          "optionalStringValue": null,
+          "requiredStringValue": "",
+        },
+      },
+    }
+  `);
+});
diff --git a/e2e/tests/pothos--proto3_optional--ts-proto/schema.ts b/e2e/tests/pothos--proto3_optional--ts-proto/schema.ts
@@ -0,0 +1,30 @@
+import { Message } from "@testapis/ts-proto/lib/testapis/proto3_optional/proto3_optional";
+
+import { printGraphqlSchema } from "../../src/printGraphqlSchema";
+import { Message$Ref } from "../__generated__/pothos/ts-proto/testapis/proto3_optional/proto3_optional.pb.pothos";
+import { builder } from "./builder";
+
+builder.queryField("valuesArePresent", (f) =>
+  f.field({
+    type: Message$Ref,
+    resolve() {
+      return Message.fromPartial({
+        requiredStringValue: "required field",
+        optionalStringValue: "optional field",
+      });
+    },
+  })
+);
+
+builder.queryField("valuesAreBlank", (f) =>
+  f.field({
+    type: Message$Ref,
+    resolve() {
+      return Message.fromPartial({});
+    },
+  })
+);
+
+export const schema = builder.toSchema();
+
+printGraphqlSchema({ schema, rootDir: __dirname });
diff --git a/e2e/tests/pothos--proto3_optional--ts-proto/tsconfig.json b/e2e/tests/pothos--proto3_optional--ts-proto/tsconfig.json
@@ -0,0 +1,17 @@
+// Code generated by setupTests.mjs. DO NOT EDIT.
+{
+  "extends": "../../tsconfig.base.json",
+  "compilerOptions": {
+    "baseUrl": ".",
+    "paths": {
+      "@testapp/*": [
+        "./*"
+      ]
+    }
+  },
+  "include": [
+    "./**/*",
+    "../../src/**/*",
+    "../__generated__/pothos/ts-proto/testapis/proto3_optional/**/*"
+  ]
+}
diff --git a/e2e/tsconfig.json b/e2e/tsconfig.json
@@ -68,6 +68,12 @@
     {
       "path": "tests/nexus--primitives--protobufjs"
     },
+    {
+      "path": "tests/nexus--proto3_optional--google-protobuf"
+    },
+    {
+      "path": "tests/nexus--proto3_optional--protobufjs"
+    },
     {
       "path": "tests/nexus--wktypes--google-protobuf"
     },
@@ -110,6 +116,9 @@
     {
       "path": "tests/pothos--primitives--ts-proto-with-forcelong-number"
     },
+    {
+      "path": "tests/pothos--proto3_optional--ts-proto"
+    },
     {
       "path": "tests/pothos--wktypes--ts-proto"
     },
diff --git a/packages/@proto-graphql/codegen-core/src/types/ObjectType.ts b/packages/@proto-graphql/codegen-core/src/types/ObjectType.ts
@@ -11,7 +11,7 @@ export class ObjectType extends TypeBase<ProtoMessage> {
   get fields(): (ObjectField<any> | ObjectOneofField)[] {
     return [
       ...this.proto.fields
-        .filter((f) => f.containingOneof == null)
+        .filter((f) => f.containingOneof == null || f.containingOneof.synthetic)
         .filter((f) => !isInputOnlyField(f))
         .filter((f) => !isIgnoredField(f))
         .map(
diff --git a/packages/@proto-graphql/codegen-core/src/types/util.ts b/packages/@proto-graphql/codegen-core/src/types/util.ts
@@ -200,6 +200,7 @@ export function isRequiredField(
     }
   }
   if (fieldType === "partial_input") return false;
+  if (field.kind === "Field" && field.optional) return false;
   return hasRequiredLabel(field);
 }
 
@@ -239,7 +240,7 @@ export function isIgnoredField(
       return true;
     }
     const oneof = field.containingOneof;
-    if (oneof && isIgnoredField(oneof)) {
+    if (oneof && !oneof.synthetic && isIgnoredField(oneof)) {
       return true;
     }
     ext = getExtension(field, "field");
@@ -249,6 +250,9 @@ export function isIgnoredField(
     if (isIgnoredType(field.parent)) {
       return true;
     }
+    if (field.synthetic) {
+      return true;
+    }
     ext = getExtension(field, "oneof");
   } else {
     const _exhaustiveCheck: never = field;
diff --git a/packages/@proto-graphql/proto-descriptors/src/impls.ts b/packages/@proto-graphql/proto-descriptors/src/impls.ts
@@ -325,6 +325,11 @@ export class ProtoOneofImpl implements ProtoOneof {
   get deprecated(): boolean {
     return isDeprecated(this);
   }
+
+  @memo()
+  get synthetic(): boolean {
+    return this.fields.length === 1 && this.fields[0].optional;
+  }
 }
 
 export class ProtoFieldImpl implements ProtoField {
@@ -385,6 +390,11 @@ export class ProtoFieldImpl implements ProtoField {
     );
   }
 
+  @memo()
+  get optional(): boolean {
+    return this.descriptor.getProto3Optional() ?? false;
+  }
+
   @memo()
   get comments(): CommentSet {
     return getCommentSetByDescriptors(this);
diff --git a/packages/@proto-graphql/proto-descriptors/src/interfaces.ts b/packages/@proto-graphql/proto-descriptors/src/interfaces.ts
@@ -137,6 +137,11 @@ export interface ProtoOneof extends ProtoBase<"Oneof"> {
   readonly fields: ProtoField[];
   readonly comments: CommentSet;
   readonly deprecated: boolean;
+  /**
+   * return true if this is synthetic oneof for backward compat for proto3 optional.
+   * @see https://github.com/protocolbuffers/protobuf/blob/v21.12/docs/implementing_proto3_presence.md#updating-a-code-generator
+   */
+  readonly synthetic: boolean;
 }
 
 export interface ProtoField extends ProtoBase<"Field"> {
@@ -158,6 +163,7 @@ export interface ProtoField extends ProtoBase<"Field"> {
   readonly type: ProtoMessage | ProtoEnum | ProtoScalar;
   readonly containingOneof: ProtoOneof | null;
   readonly list: boolean;
+  readonly optional: boolean;
   readonly comments: CommentSet;
   readonly deprecated: boolean;
 }
diff --git a/packages/@testapis/proto/src/testapis/proto3_optional/descriptor_set.pb b/packages/@testapis/proto/src/testapis/proto3_optional/descriptor_set.pb
diff --git a/packages/@testapis/proto/src/testapis/proto3_optional/proto3_optional.proto b/packages/@testapis/proto/src/testapis/proto3_optional/proto3_optional.proto
@@ -0,0 +1,8 @@
+syntax = "proto3";
+
+package testapis.proto3_optional;
+
+message Message {
+  string required_string_value = 1;
+  optional string optional_string_value = 2;
+}
diff --git a/packages/protoc-gen-nexus/src/__tests__/__snapshots__/proto3Optional.test.ts.snap b/packages/protoc-gen-nexus/src/__tests__/__snapshots__/proto3Optional.test.ts.snap
diff --git a/packages/protoc-gen-nexus/src/__tests__/proto3Optional.test.ts b/packages/protoc-gen-nexus/src/__tests__/proto3Optional.test.ts
diff --git a/packages/protoc-gen-pothos/src/__tests__/__snapshots__/proto3Optional.test.ts.snap b/packages/protoc-gen-pothos/src/__tests__/__snapshots__/proto3Optional.test.ts.snap
diff --git a/packages/protoc-gen-pothos/src/__tests__/proto3Optional.test.ts b/packages/protoc-gen-pothos/src/__tests__/proto3Optional.test.ts
