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

Allow separate generation of test clients #881

Merged
merged 1 commit into from
Jul 7, 2020

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Jul 6, 2020

Motivation:

Users should be able to generate their test clients separately to their
real clients so that their real clients can be public and their test
clients internal.

Modifications:

  • Allow test clients to be generated separately from real clients
  • Add an option to control whether conformance is generated; this allows
    test clients to be generated separately from real clients when the
    service and messages are defined in a single file without duplicating
    the conformance.
  • Expand on the plugin documentation.

Result:

Resolves #878

Motivation:

Users should be able to generate their test clients separately to their
real clients so that their real clients can be public and their test
clients internal.

Modifications:

- Allow test clients to be generated separately from real clients
- Add an option to control whether conformance is generated; this allows
  test clients to be generated separately from real clients when the
  service and messages are defined in a single file without duplicating
  the conformance.
- Expand on the plugin documentation.

Result:

Resolves grpc#878
@glbrntt glbrntt added 🔨 semver/patch No public API change. codegen Relates to the protoc-gen-grpc-swift plugin labels Jul 6, 2020
@glbrntt glbrntt requested a review from Lukasa July 6, 2020 16:18
Copy link
Collaborator

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

LGTM, nice change.

@glbrntt glbrntt merged commit 2272cc9 into grpc:master Jul 7, 2020
@glbrntt glbrntt deleted the gb-generate-test-client-separately branch July 7, 2020 09:43
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Jul 10, 2020
…rver

Motivation:

To support payloads other than `SwiftProtobuf.Message` we required that
all messages conform to `GRPCPayload`. For protobuf messages we added
`GRPCProtobufPayload` which provides a default implemenation of
`GRPCPayload` for protobuf messages. We generated this conformance for
all protobuf messages we saw. This lead to a number issues and
workarounds including: grpc#738, grpc#778, grpc#801, grpc#837, grpc#877, grpc#881.

The intention is to continue to support `GRPCPayload` in addition to
protobuf, however, support for protobuf will not be via the
`GRPCProtobufPayload` protocol.

This PR adjust the server components such they only support
SwiftProtobuf. Once the client side has had the same treatment (and
`GRPCProtobufPayload` no longer inherits from `SwiftProtobuf.Message`),
support for `GRPCPayload` will be added back.

Modifications:

- The `HTTP1ToGRPCServerCodec` has had the message encoding and decoding
  removed. It now deals in `ByteBuffer`s rather than request/response
  messages.
- An additional `GRPCServerCodecHandler` which sits between the
  `HTTP1ToGRPCServerCodec` and `_BaseCallHandler` has been added which
  serializes/deserializes messages.
- Custom payload tests have been commented out. They will return when
  the transition has completed.

Result:

- Servers only support SwiftProtobuf
- Genertic constraints on the server have been removed; the constraints
  are place on the `init` of public handlers instead.
- `GRPCProtobufPayload` is no longer required on the server.
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Jul 10, 2020
…rver

Motivation:

To support payloads other than `SwiftProtobuf.Message` we required that
all messages conform to `GRPCPayload`. For protobuf messages we added
`GRPCProtobufPayload` which provides a default implemenation of
`GRPCPayload` for protobuf messages. We generated this conformance for
all protobuf messages we saw. This lead to a number issues and
workarounds including: grpc#738, grpc#778, grpc#801, grpc#837, grpc#877, grpc#881.

The intention is to continue to support `GRPCPayload` in addition to
protobuf, however, support for protobuf will not be via the
`GRPCProtobufPayload` protocol.

This PR adjusts server components such that they are not constrained to
`GRPCPayload`. At the moment only `SwiftProtobuf.Message` is supported.
Once the client side has had the same treatment and
`GRPCProtobufPayload` no longer inherits from `SwiftProtobuf.Message`,
support for `GRPCPayload` will be added back.

Modifications:

- The `HTTP1ToGRPCServerCodec` has had the message encoding and decoding
  removed. It now deals in `ByteBuffer`s rather than request/response
  messages.
- An additional `GRPCServerCodecHandler` which sits between the
  `HTTP1ToGRPCServerCodec` and `_BaseCallHandler` has been added which
  serializes/deserializes messages.
- Custom payload tests have been commented out. They will return when
  the transition has completed.

Result:

- Servers only support SwiftProtobuf
- Genertic constraints on the server have been removed; the constraints
  are place on the `init` of public handlers instead.
- `GRPCProtobufPayload` is no longer required on the server.
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Jul 10, 2020
…rver

Motivation:

To support payloads other than `SwiftProtobuf.Message` we required that
all messages conform to `GRPCPayload`. For protobuf messages we added
`GRPCProtobufPayload` which provides a default implemenation of
`GRPCPayload` for protobuf messages. We generated this conformance for
all protobuf messages we saw. This lead to a number issues and
workarounds including: grpc#738, grpc#778, grpc#801, grpc#837, grpc#877, grpc#881.

The intention is to continue to support `GRPCPayload` in addition to
protobuf, however, support for protobuf will not be via the
`GRPCProtobufPayload` protocol.

This PR adjusts server components such that they are not constrained to
`GRPCPayload`. At the moment only `SwiftProtobuf.Message` is supported.
Once the client side has had the same treatment and
`GRPCProtobufPayload` no longer inherits from `SwiftProtobuf.Message`,
support for `GRPCPayload` will be added back.

Modifications:

- The `HTTP1ToGRPCServerCodec` has had the message encoding and decoding
  removed. It now deals in `ByteBuffer`s rather than request/response
  messages.
- An additional `GRPCServerCodecHandler` which sits between the
  `HTTP1ToGRPCServerCodec` and `_BaseCallHandler` has been added which
  serializes/deserializes messages.
- Custom payload tests have been commented out. They will return when
  the transition has completed.

Result:

- Servers only support SwiftProtobuf
- Genertic constraints on the server have been removed; the constraints
  are place on the `init` of public handlers instead.
- `GRPCProtobufPayload` is no longer required on the server.
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Jul 10, 2020
…rver

Motivation:

To support payloads other than `SwiftProtobuf.Message` we required that
all messages conform to `GRPCPayload`. For protobuf messages we added
`GRPCProtobufPayload` which provides a default implemenation of
`GRPCPayload` for protobuf messages. We generated this conformance for
all protobuf messages we saw. This lead to a number issues and
workarounds including: grpc#738, grpc#778, grpc#801, grpc#837, grpc#877, grpc#881.

The intention is to continue to support `GRPCPayload` in addition to
protobuf, however, support for protobuf will not be via the
`GRPCProtobufPayload` protocol.

This PR adjusts server components such that they are not constrained to
`GRPCPayload`. At the moment only `SwiftProtobuf.Message` is supported.
Once the client side has had the same treatment and
`GRPCProtobufPayload` no longer inherits from `SwiftProtobuf.Message`,
support for `GRPCPayload` will be added back.

Modifications:

- The `HTTP1ToGRPCServerCodec` has had the message encoding and decoding
  removed. It now deals in `ByteBuffer`s rather than request/response
  messages.
- An additional `GRPCServerCodecHandler` which sits between the
  `HTTP1ToGRPCServerCodec` and `_BaseCallHandler` has been added which
  serializes/deserializes messages.
- Custom payload tests have been commented out. They will return when
  the transition has completed.

Result:

- Servers only support SwiftProtobuf
- Genertic constraints on the server have been removed; the constraints
  are place on the `init` of public handlers instead.
- `GRPCProtobufPayload` is no longer required on the server.
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Jul 10, 2020
…rver

Motivation:

To support payloads other than `SwiftProtobuf.Message` we required that
all messages conform to `GRPCPayload`. For protobuf messages we added
`GRPCProtobufPayload` which provides a default implemenation of
`GRPCPayload` for protobuf messages. We generated this conformance for
all protobuf messages we saw. This lead to a number issues and
workarounds including: grpc#738, grpc#778, grpc#801, grpc#837, grpc#877, grpc#881.

The intention is to continue to support `GRPCPayload` in addition to
protobuf, however, support for protobuf will not be via the
`GRPCProtobufPayload` protocol.

This PR adjusts server components such that they are not constrained to
`GRPCPayload`. At the moment only `SwiftProtobuf.Message` is supported.
Once the client side has had the same treatment and
`GRPCProtobufPayload` no longer inherits from `SwiftProtobuf.Message`,
support for `GRPCPayload` will be added back.

Modifications:

- The `HTTP1ToGRPCServerCodec` has had the message encoding and decoding
  removed. It now deals in `ByteBuffer`s rather than request/response
  messages.
- An additional `GRPCServerCodecHandler` which sits between the
  `HTTP1ToGRPCServerCodec` and `_BaseCallHandler` has been added which
  serializes/deserializes messages.
- Custom payload tests have been commented out. They will return when
  the transition has completed.

Result:

- Servers only support SwiftProtobuf
- Genertic constraints on the server have been removed; the constraints
  are place on the `init` of public handlers instead.
- `GRPCProtobufPayload` is no longer required on the server.
glbrntt added a commit that referenced this pull request Jul 13, 2020
…rver (#886)

Motivation:

To support payloads other than `SwiftProtobuf.Message` we required that
all messages conform to `GRPCPayload`. For protobuf messages we added
`GRPCProtobufPayload` which provides a default implemenation of
`GRPCPayload` for protobuf messages. We generated this conformance for
all protobuf messages we saw. This lead to a number issues and
workarounds including: #738, #778, #801, #837, #877, #881.

The intention is to continue to support `GRPCPayload` in addition to
protobuf, however, support for protobuf will not be via the
`GRPCProtobufPayload` protocol.

This PR adjusts server components such that they are not constrained to
`GRPCPayload`. At the moment only `SwiftProtobuf.Message` is supported.
Once the client side has had the same treatment and
`GRPCProtobufPayload` no longer inherits from `SwiftProtobuf.Message`,
support for `GRPCPayload` will be added back.

Modifications:

- The `HTTP1ToGRPCServerCodec` has had the message encoding and decoding
  removed. It now deals in `ByteBuffer`s rather than request/response
  messages.
- An additional `GRPCServerCodecHandler` which sits between the
  `HTTP1ToGRPCServerCodec` and `_BaseCallHandler` has been added which
  serializes/deserializes messages.
- Custom payload tests have been commented out. They will return when
  the transition has completed.

Result:

- Servers only support SwiftProtobuf
- Genertic constraints on the server have been removed; the constraints
  are place on the `init` of public handlers instead.
- `GRPCProtobufPayload` is no longer required on the server.
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Jul 14, 2020
Motivation:

To support payloads other than `SwiftProtobuf.Message` we required that
all messages conform to `GRPCPayload`. For protobuf messages we added
`GRPCProtobufPayload` which provides a default implemenation of
`GRPCPayload` for protobuf messages. We generated this conformance for
all protobuf messages we saw. This lead to a number issues and
workarounds including: grpc#738, grpc#778, grpc#801, grpc#837, grpc#877, grpc#881.

The intention is to continue to support `GRPCPayload` in addition to
protobuf, however, support for protobuf will not be via the
`GRPCProtobufPayload` protocol.

This PR builds on grpc#886 by increasing the surface area of the client APIs
so that they are not constrained to `GRPCPayload`. The surface API now
has variants for `GRPCPayload` and `SwiftProtobuf.Message`. Internally
the client deals with serializers and deserializers.

Modifications:

- `GRPCClientChannelHandler` and `GRPCClientStateMachine` are no longer
  generic over a request and response type, rather they deal with the
  serialzed version of requests and response (i.e. `ByteBuffer`s) and
  defer the (de/)serialization to a separate handler.
- Added `GRCPClientCodecHandler` to handle (de/)serialization of
  messages
- Clients are no longer constrained to having their request/response
  payloads conform to `GRPCPayload`
- Conformance to `GRPCProtobufPayload` is no longer generated and the
  protocol is deprecated and has no requirements.
- Drop the 'GenerateConformance' option from the codegen since it is no
  longer required
- Reintroduce a filter to the codegen so that we only consider files
  which contain services, this avoids generating empty files
- Regenerate code where necessary

Result:

- `GRPCProtobufPayload` is no longer required
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Jul 14, 2020
Motivation:

To support payloads other than `SwiftProtobuf.Message` we required that
all messages conform to `GRPCPayload`. For protobuf messages we added
`GRPCProtobufPayload` which provides a default implemenation of
`GRPCPayload` for protobuf messages. We generated this conformance for
all protobuf messages we saw. This lead to a number issues and
workarounds including: grpc#738, grpc#778, grpc#801, grpc#837, grpc#877, grpc#881.

The intention is to continue to support `GRPCPayload` in addition to
protobuf, however, support for protobuf will not be via the
`GRPCProtobufPayload` protocol.

This PR builds on grpc#886 by increasing the surface area of the client APIs
so that they are not constrained to `GRPCPayload`. The surface API now
has variants for `GRPCPayload` and `SwiftProtobuf.Message`. Internally
the client deals with serializers and deserializers.

Modifications:

- `GRPCClientChannelHandler` and `GRPCClientStateMachine` are no longer
  generic over a request and response type, rather they deal with the
  serialzed version of requests and response (i.e. `ByteBuffer`s) and
  defer the (de/)serialization to a separate handler.
- Added `GRCPClientCodecHandler` to handle (de/)serialization of
  messages
- Clients are no longer constrained to having their request/response
  payloads conform to `GRPCPayload`
- Conformance to `GRPCProtobufPayload` is no longer generated and the
  protocol is deprecated and has no requirements.
- Drop the 'GenerateConformance' option from the codegen since it is no
  longer required
- Reintroduce a filter to the codegen so that we only consider files
  which contain services, this avoids generating empty files
- Regenerate code where necessary

Result:

- `GRPCProtobufPayload` is no longer required
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Jul 14, 2020
Motivation:

To support payloads other than `SwiftProtobuf.Message` we required that
all messages conform to `GRPCPayload`. For protobuf messages we added
`GRPCProtobufPayload` which provides a default implemenation of
`GRPCPayload` for protobuf messages. We generated this conformance for
all protobuf messages we saw. This lead to a number issues and
workarounds including: grpc#738, grpc#778, grpc#801, grpc#837, grpc#877, grpc#881.

The intention is to continue to support `GRPCPayload` in addition to
protobuf, however, support for protobuf will not be via the
`GRPCProtobufPayload` protocol.

This PR builds on grpc#886 by increasing the surface area of the client APIs
so that they are not constrained to `GRPCPayload`. The surface API now
has variants for `GRPCPayload` and `SwiftProtobuf.Message`. Internally
the client deals with serializers and deserializers.

Modifications:

- `GRPCClientChannelHandler` and `GRPCClientStateMachine` are no longer
  generic over a request and response type, rather they deal with the
  serialzed version of requests and response (i.e. `ByteBuffer`s) and
  defer the (de/)serialization to a separate handler.
- Added `GRCPClientCodecHandler` to handle (de/)serialization of
  messages
- Clients are no longer constrained to having their request/response
  payloads conform to `GRPCPayload`
- Conformance to `GRPCProtobufPayload` is no longer generated and the
  protocol is deprecated and has no requirements.
- Drop the 'GenerateConformance' option from the codegen since it is no
  longer required
- Reintroduce a filter to the codegen so that we only consider files
  which contain services, this avoids generating empty files
- Regenerate code where necessary

Result:

- `GRPCProtobufPayload` is no longer required
glbrntt added a commit that referenced this pull request Jul 14, 2020
Motivation:

To support payloads other than `SwiftProtobuf.Message` we required that
all messages conform to `GRPCPayload`. For protobuf messages we added
`GRPCProtobufPayload` which provides a default implemenation of
`GRPCPayload` for protobuf messages. We generated this conformance for
all protobuf messages we saw. This lead to a number issues and
workarounds including: #738, #778, #801, #837, #877, #881.

The intention is to continue to support `GRPCPayload` in addition to
protobuf, however, support for protobuf will not be via the
`GRPCProtobufPayload` protocol.

This PR builds on #886 by increasing the surface area of the client APIs
so that they are not constrained to `GRPCPayload`. The surface API now
has variants for `GRPCPayload` and `SwiftProtobuf.Message`. Internally
the client deals with serializers and deserializers.

Modifications:

- `GRPCClientChannelHandler` and `GRPCClientStateMachine` are no longer
  generic over a request and response type, rather they deal with the
  serialzed version of requests and response (i.e. `ByteBuffer`s) and
  defer the (de/)serialization to a separate handler.
- Added `GRCPClientCodecHandler` to handle (de/)serialization of
  messages
- Clients are no longer constrained to having their request/response
  payloads conform to `GRPCPayload`
- Conformance to `GRPCProtobufPayload` is no longer generated and the
  protocol is deprecated and has no requirements.
- Drop the 'GenerateConformance' option from the codegen since it is no
  longer required
- Reintroduce a filter to the codegen so that we only consider files
  which contain services, this avoids generating empty files
- Regenerate code where necessary

Result:

- `GRPCProtobufPayload` is no longer required
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codegen Relates to the protoc-gen-grpc-swift plugin 🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ability to put generated test clients into separate files from their production equivalents
2 participants