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

Framework: Refs with imposed strong types lead to ClassCastException #619

Closed
krestjaninoff opened this issue Aug 23, 2018 · 3 comments · Fixed by #627
Closed

Framework: Refs with imposed strong types lead to ClassCastException #619

krestjaninoff opened this issue Aug 23, 2018 · 3 comments · Fixed by #627
Labels
bug This issue is a bug.

Comments

@krestjaninoff
Copy link

Hello,

I'm trying to create a simple VPC with a single subnet with predefined CIDR using Java:

        VpcNetwork vpc = new VpcNetwork(this, "my-vpc",
                VpcNetworkProps.builder()
                        .withCidr("10.0.0.0/16")
                        .build());
        Token vpcId = vpc.getVpcId();

        new VpcSubnet(this, "my-vpc-a",
                VpcSubnetProps.builder()
                        .withAvailabilityZone(props.getEnv().getRegion() + "a")
                        .withVpcId(vpcId)
                        .withCidrBlock("10.0.1.0/24")
                        .build()

Unfortunately, cdr synth fails with the following error:

Exception in thread "main" java.lang.ClassCastException: software.amazon.awscdk.CloudFormationToken cannot be cast to software.amazon.awscdk.services.ec2.VpcNetworkId
     at software.amazon.awscdk.services.ec2.VpcNetwork.getVpcId(VpcNetwork.java:67)

when vpc.getVpcId() is invoked.

It looks like a bug on Jsii but maybe I'm missing something.


CDK Version: 0.8

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 23, 2018

You're not missing anything. Unfortunately, this cannot work as written right now.

@rix0rrr rix0rrr changed the title ClassCastException when trying to create VpcSubnet Framework: Refs with imposed type names lead to ClassCastException Aug 23, 2018
@rix0rrr rix0rrr added the bug This issue is a bug. label Aug 23, 2018
@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 23, 2018

Here's the issue. This is our inheritance hierarchy:

              ┌───────────────┐             
              │               │             
              │     Token     │             
              │               │             
              └───────────────┘             
                      △                     
                      │                     
        ┌─────────────┴────────────┐        
        │                          │        
        │                          │        
┌───────────────┐          ┌───────────────┐
│               │          │               │
│      Ref      │          │     VpcId     │
│               │          │               │
└───────────────┘          └───────────────┘

At some point, we're making the following assignment:

this.vpcId = vpc.ref;

Apparently, TypeScript allows this because of the structural type checking it employs.

Java will do nominal type checking however, and will throw the ClassCastException once you try to put a Ref instance into a variable of type VpcId.

EDIT: Actually, the terminology is not quite correct. What's called Ref in that diagram is actually CloudFormationToken, which is fairly new. It used to be just Token, but even that would not have worked in Java.

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 23, 2018

An easy fix for this is to change into:

this.vpcId = new VpcId(vpc.ref);

However, I think we might want to annotate the CloudFormation spec instead and resolve it there.

@rix0rrr rix0rrr changed the title Framework: Refs with imposed type names lead to ClassCastException Framework: Refs with imposed strong types lead to ClassCastException Aug 23, 2018
rix0rrr added a commit that referenced this issue Aug 27, 2018
Annotate all CloudFormation resource types with the type of their 'Ref'
(whether it returns a `Name`, `Id` or `Arn`). We generate specific
classes for those types, just like we do for `{Fn::GetAtt}` attributes.

This makes it easier to write construct libraries: it removes the need
for every construct library to explicitly declare a custom type for the 
ref implicit type, and reduces chances of a `ClassCastException` in Java
if they do it wrong.

Generated resource classes no longer implicitly inherit from
`Referenceable`, because not all resources even have the `{Ref}`
operator defined.

Fixes #619.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants