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

revert(rds): document DatabaseInstance.fromDatabaseInstanceAttributes() #12091

Merged
merged 4 commits into from
Dec 15, 2020

Conversation

iliapolo
Copy link
Contributor

@iliapolo iliapolo commented Dec 15, 2020

Reverts #11979

This commit broke java compilation of the module:

#STDOUT> [ERROR] COMPILATION ERROR : 
#STDOUT> [INFO] -------------------------------------------------------------
#STDOUT> [ERROR] /tmp/npm-packP61iji/monocdk/src/main/java/software/amazon/awscdk/services/rds/DatabaseInstance.java:[38,5] method does not override or implement a method from a supertype

Basically the fromDatabaseInstanceAttributes function now exists both in DatabaseInstanceBase and DatabaseInstance, and for some reason the one in the child is annotated with an @Override, despite being a static function. (see more details below).

Since this module is stable, I don't want to simply move the function, but I do want to bring our pipeline back to life quickly, so this seems like an ok compromise for now.


What happens is that the generated java code for some reason adds an @Override annotation to the function signature of the concrete class, as if its overriding the function in the base class, even though these are static methods.

@javax.annotation.Generated(value = "jsii-pacmak/1.15.0 (build 585166b)", date = "2020-12-15T17:40:10.678Z")
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Stable)
@software.amazon.jsii.Jsii(module = software.amazon.awscdk.services.rds.$Module.class, fqn = "@aws-cdk/aws-rds.DatabaseInstance")
public class DatabaseInstance extends software.amazon.awscdk.services.rds.DatabaseInstanceBase implements software.amazon.awscdk.services.rds.IDatabaseInstance {

    protected DatabaseInstance(final software.amazon.jsii.JsiiObjectRef objRef) {
        super(objRef);
    }

    protected DatabaseInstance(final software.amazon.jsii.JsiiObject.InitializationMode initializationMode) {
        super(initializationMode);
    }

    /**
     * @param scope This parameter is required.
     * @param id This parameter is required.
     * @param props This parameter is required.
     */
    @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Stable)
    public DatabaseInstance(final @org.jetbrains.annotations.NotNull software.constructs.Construct scope, final @org.jetbrains.annotations.NotNull java.lang.String id, final @org.jetbrains.annotations.NotNull software.amazon.awscdk.services.rds.DatabaseInstanceProps props) {
        super(software.amazon.jsii.JsiiObject.InitializationMode.JSII);
        software.amazon.jsii.JsiiEngine.getInstance().createNewObject(this, new Object[] { java.util.Objects.requireNonNull(scope, "scope is required"), java.util.Objects.requireNonNull(id, "id is required"), java.util.Objects.requireNonNull(props, "props is required") });
    }

    /**
     * Import an existing database instance.
     * <p>
     * @param scope This parameter is required.
     * @param id This parameter is required.
     * @param attrs This parameter is required.
     */
    @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Stable)
    @Override
    public static @org.jetbrains.annotations.NotNull software.amazon.awscdk.services.rds.IDatabaseInstance fromDatabaseInstanceAttributes(final @org.jetbrains.annotations.NotNull software.constructs.Construct scope, final @org.jetbrains.annotations.NotNull java.lang.String id, final @org.jetbrains.annotations.NotNull software.amazon.awscdk.services.rds.DatabaseInstanceAttributes attrs) {
        return software.amazon.jsii.JsiiObject.jsiiStaticCall(software.amazon.awscdk.services.rds.DatabaseInstance.class, "fromDatabaseInstanceAttributes", software.amazon.jsii.NativeType.forClass(software.amazon.awscdk.services.rds.IDatabaseInstance.class), new Object[] { java.util.Objects.requireNonNull(scope, "scope is required"), java.util.Objects.requireNonNull(id, "id is required"), java.util.Objects.requireNonNull(attrs, "attrs is required") });
    }

JSII assembly also shows:

"name": "fromDatabaseInstanceAttributes",
"overrides": "@aws-cdk/aws-rds.DatabaseInstanceBase",
"parameters": [
  {
    "name": "scope",
    "type": {
      "fqn": "constructs.Construct"
    }
  },
  {
    "name": "id",
    "type": {
      "primitive": "string"
    }
  },
  {
    "name": "attrs",
    "type": {
      "fqn": "@aws-cdk/aws-rds.DatabaseInstanceAttributes"
    }
  }
],
"returns": {
  "type": {
    "fqn": "@aws-cdk/aws-rds.IDatabaseInstance"
  }
},
"static": true

This feels like a JSII bug, it shouldn't be marking this method as an override in the assembly, and regardless, it should not generate java code that has an @Override annotation on a static function, better to validate and throw beforehand.

I guess this is related to Typescript quirks with inheritance of static members?

cc @RomainMuller @skinny85

@gitpod-io
Copy link

gitpod-io bot commented Dec 15, 2020

@github-actions github-actions bot added the @aws-cdk/aws-rds Related to Amazon Relational Database label Dec 15, 2020
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Dec 15, 2020
@mergify
Copy link
Contributor

mergify bot commented Dec 15, 2020

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

@mergify
Copy link
Contributor

mergify bot commented Dec 15, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@iliapolo iliapolo changed the title Revert "chore(rds): document DatabaseInstance.fromDatabaseInstanceAttributes()" revert(rds): document DatabaseInstance.fromDatabaseInstanceAttributes() Dec 15, 2020
@mergify
Copy link
Contributor

mergify bot commented Dec 15, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify
Copy link
Contributor

mergify bot commented Dec 15, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 054610f into master Dec 15, 2020
@mergify mergify bot deleted the revert-11979-ktheory/11817 branch December 15, 2020 20:03
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: a080dfc
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

flochaz pushed a commit to flochaz/aws-cdk that referenced this pull request Jan 5, 2021
…() (aws#12091)

Reverts aws#11979

This commit broke java compilation of the module:

```console
#STDOUT> [ERROR] COMPILATION ERROR : 
#STDOUT> [INFO] -------------------------------------------------------------
#STDOUT> [ERROR] /tmp/npm-packP61iji/monocdk/src/main/java/software/amazon/awscdk/services/rds/DatabaseInstance.java:[38,5] method does not override or implement a method from a supertype
```

Basically the `fromDatabaseInstanceAttributes` function now exists both in `DatabaseInstanceBase` and `DatabaseInstance`, and for some reason the one in the child is annotated with an `@Override`, despite being a static function. (see more details below).

Since this module is stable, I don't want to simply move the function, but I do want to bring our pipeline back to life quickly, so this seems like an ok compromise for now.

-----------------

What happens is that the generated java code for some reason adds an `@Override` annotation to the function signature of the concrete class, as if its overriding the function in the base class, even though these are static methods.

```java
@javax.annotation.Generated(value = "jsii-pacmak/1.15.0 (build 585166b)", date = "2020-12-15T17:40:10.678Z")
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Stable)
@software.amazon.jsii.Jsii(module = software.amazon.awscdk.services.rds.$Module.class, fqn = "@aws-cdk/aws-rds.DatabaseInstance")
public class DatabaseInstance extends software.amazon.awscdk.services.rds.DatabaseInstanceBase implements software.amazon.awscdk.services.rds.IDatabaseInstance {

    protected DatabaseInstance(final software.amazon.jsii.JsiiObjectRef objRef) {
        super(objRef);
    }

    protected DatabaseInstance(final software.amazon.jsii.JsiiObject.InitializationMode initializationMode) {
        super(initializationMode);
    }

    /**
     * @param scope This parameter is required.
     * @param id This parameter is required.
     * @param props This parameter is required.
     */
    @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Stable)
    public DatabaseInstance(final @org.jetbrains.annotations.NotNull software.constructs.Construct scope, final @org.jetbrains.annotations.NotNull java.lang.String id, final @org.jetbrains.annotations.NotNull software.amazon.awscdk.services.rds.DatabaseInstanceProps props) {
        super(software.amazon.jsii.JsiiObject.InitializationMode.JSII);
        software.amazon.jsii.JsiiEngine.getInstance().createNewObject(this, new Object[] { java.util.Objects.requireNonNull(scope, "scope is required"), java.util.Objects.requireNonNull(id, "id is required"), java.util.Objects.requireNonNull(props, "props is required") });
    }

    /**
     * Import an existing database instance.
     * <p>
     * @param scope This parameter is required.
     * @param id This parameter is required.
     * @param attrs This parameter is required.
     */
    @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Stable)
    @OverRide
    public static @org.jetbrains.annotations.NotNull software.amazon.awscdk.services.rds.IDatabaseInstance fromDatabaseInstanceAttributes(final @org.jetbrains.annotations.NotNull software.constructs.Construct scope, final @org.jetbrains.annotations.NotNull java.lang.String id, final @org.jetbrains.annotations.NotNull software.amazon.awscdk.services.rds.DatabaseInstanceAttributes attrs) {
        return software.amazon.jsii.JsiiObject.jsiiStaticCall(software.amazon.awscdk.services.rds.DatabaseInstance.class, "fromDatabaseInstanceAttributes", software.amazon.jsii.NativeType.forClass(software.amazon.awscdk.services.rds.IDatabaseInstance.class), new Object[] { java.util.Objects.requireNonNull(scope, "scope is required"), java.util.Objects.requireNonNull(id, "id is required"), java.util.Objects.requireNonNull(attrs, "attrs is required") });
    }
```

JSII assembly also shows:

```json
"name": "fromDatabaseInstanceAttributes",
"overrides": "@aws-cdk/aws-rds.DatabaseInstanceBase",
"parameters": [
  {
    "name": "scope",
    "type": {
      "fqn": "constructs.Construct"
    }
  },
  {
    "name": "id",
    "type": {
      "primitive": "string"
    }
  },
  {
    "name": "attrs",
    "type": {
      "fqn": "@aws-cdk/aws-rds.DatabaseInstanceAttributes"
    }
  }
],
"returns": {
  "type": {
    "fqn": "@aws-cdk/aws-rds.IDatabaseInstance"
  }
},
"static": true
```

This feels like a JSII bug, it shouldn't be marking this method as an override in the assembly, and regardless, it should not generate java code that has an `@Override` annotation on a static function, better to validate and throw beforehand.

I guess this is related to Typescript quirks with inheritance of static members?

cc @RomainMuller @skinny85
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants