fix(aws-events-targets): Consume IRestApi as target

[AWS-MattB]

Fixes: #16423

When creating an Event Bridge target for an API Gateway, the
superinterface IRestApi should be consumed instead of the
concrete class RestApi.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io]

[rix0rrr]

The error is:

API elements with incompatible changes:
err  - PROP @aws-cdk/aws-events-targets.ApiGateway.restApi: type @aws-cdk/aws-apigateway.IRestApi (formerly @aws-cdk/aws-apigateway.RestApi): @aws-cdk/aws-apigateway.IRestApi does not extend @aws-cdk/aws-apigateway.RestApi [changed-type:@aws-cdk/aws-events-targets.ApiGateway.restApi]

The reason why this change is incompatible is because public readonly restApi declares both an INPUT parameter and an OUTPUT parameter, and we can only change one in one direction, and the other in the other. That makes it somewhat more complex, so It probably needs to look somewhat like this:

private readonly _restApi: IRestApi;

constructor(restApi: IRestApi, private readonly props?: ApiGatewayProps) {
  this._restApi = resetApi;
}

public get restApi(): RestApi {
  if (!(this._restApi instanceof RestApi)) {
    throw new Error('The RestApi is not a RestApi construct, and cannot be retrieved this way');
  }
  return this._restApi;
}

[AWS-MattB]

Here are some possible alternatives, let me know what you prefer:

1. Add a class SpecApiGateway implements events.IRuleTarget, with a public readonly specRestApi: api.SpecRestApi, so that 100% of the known implementors of api.IRestApi are covered. Extract the common logic between the two to a private class
2. Add a non-abstract superclass of ApiGateway (open to naming suggestions) with a public readonly iRestApi: api.IRestApi, move most of the logic there. This would result in ApiGateway having two properties (restApi, iRestApi) referring to the same object, but would allow the gradual deprecation of ApiGateway in favor of the superclass.
3. Add an additional constructor which accepts public readonly iRestApi: api.IRestApi and explicitly define a getter method for restApi: api.RestApi, which throws an error if the api.IRestApi constructor was used (as you suggested above)

Pull request has been modified.

[AWS-MattB]

I've updated the PR to be option 2 (add a non-abstract superclass of ApiGateway) and have named it ApiGatewayTarget.

[AWS-MattB]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
• Commit ID: 4b31f64
• Result: FAILED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

The failure is due to a 5xx error talking to Yarn's repo. This is a rebase of the above commit, which built OK in the CI, and this commit built OK on my local environment.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
• Commit ID: be26ec3
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[rix0rrr]

I think I told you what to do in the previous review comment.

Did it not work? If so, you should tell me about it. I think I prefer that solution.

[rix0rrr]

As far as I can tell, none of the other classes end in Target.

I'm not sure this one should buck the trend.

[rix0rrr]

Suggested change

	constructor(public readonly iRestApi: api.IRestApi, protected readonly props?: ApiGatewayProps) {
	constructor(public readonly restApi: api.IRestApi, protected readonly props?: ApiGatewayProps) {

[rix0rrr]

Oh I see. No, we can repurpose this class.