Skip to content

Commit

Permalink
fix(python): unknown type when submodule is not loaded (#3049)
Browse files Browse the repository at this point in the history
Callbacks may make references to types from submodules of loaded
packages, however due to how the type registration works in the Python
runtime, if the submodule has not been loaded, then the types it
contains are not registered and will fail resolving.

The simplest possible fix is to preemtively load all submodules upfront
at the end of the root module's `__init__.py` file, which guarantees
that all types have correctly been registered in the runtime (at the
expense of some time and memory).

Fixes aws/aws-cdk#16625
  • Loading branch information
Romain Marcadier authored Oct 11, 2021
1 parent 72b9b98 commit da55a1e
Show file tree
Hide file tree
Showing 19 changed files with 988 additions and 4 deletions.
3 changes: 2 additions & 1 deletion gh-pages/content/specification/6-compliance-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
This section details the current state of each language binding with respect to our standard compliance suite.


| number | test | java (99.15%) | golang (77.97%) | Dotnet | Python |
| number | test | java (99.16%) | golang (78.15%) | Dotnet | Python |
| ------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------- | -------------------------------------------- | ------ | ------ |
| 1 | asyncOverrides_overrideCallsSuper | 🟢 | [🔴](https://github.com/aws/jsii/issues/2670) |||
| 2 | [arrayReturnedByMethodCanBeRead]("Array created in the kernel can be queried for its elements") | 🟢 | 🟢 |||
Expand Down Expand Up @@ -125,3 +125,4 @@ This section details the current state of each language binding with respect to
| 116 | subclassing | 🟢 | 🟢 |||
| 117 | testInterfaces | 🟢 | 🟢 |||
| 118 | [callbackParameterIsInterface]("Validates pure interfaces can be passed to callbacks") || 🟢 |||
| 119 | [classCanBeUsedWhenNotExpressedlyLoaded]("Validates that types not explicitly loaded by the user can safely be returned by JS code") | 🟢 | 🟢 |||
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Linq;
using Amazon.JSII.Runtime.Deputy;
using Amazon.JSII.Tests.CalculatorNamespace;
using Amazon.JSII.Tests.CalculatorNamespace.Cdk16625;
using CompositeOperation = Amazon.JSII.Tests.CalculatorNamespace.Composition.CompositeOperation;
using Amazon.JSII.Tests.CalculatorNamespace.LibNamespace;
using Amazon.JSII.Tests.CalculatorNamespace.BaseOfBaseNamespace;
Expand Down Expand Up @@ -1522,8 +1523,20 @@ public void ArrayOfInterfaces()

var allTypes = new AllTypes();
allTypes.AnyProperty = bells;

Assert.Equal(bells, allTypes.AnyProperty);
}

[Fact(DisplayName = Prefix + nameof(ClassCanBeUsedWhenNotExpressedlyLoaded))]
public void ClassCanBeUsedWhenNotExpressedlyLoaded()
{
new Cdk16625Impl().Test();
}

private sealed class Cdk16625Impl: Cdk16625 {
protected override double Unwrap(IRandomNumberGenerator rng) {
return rng.Next();
}
}
}
}
6 changes: 6 additions & 0 deletions packages/@jsii/go-runtime-test/project/compliance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/aws/jsii-runtime-go"
"github.com/aws/jsii/go-runtime-test/internal/addTen"
"github.com/aws/jsii/go-runtime-test/internal/bellRinger"
"github.com/aws/jsii/go-runtime-test/internal/cdk16625"
"github.com/aws/jsii/go-runtime-test/internal/doNotOverridePrivates"
"github.com/aws/jsii/go-runtime-test/internal/friendlyRandom"
"github.com/aws/jsii/go-runtime-test/internal/overrideAsyncMethods"
Expand Down Expand Up @@ -1640,6 +1641,11 @@ func (suite *ComplianceSuite) TestCallbackParameterIsInterface() {
require.True(*calc.ConsumerCanRingBell_StaticImplementedByPublicClass(ringer))
}

func (suite *ComplianceSuite) TestClassCanBeUsedWhenNotExpressedlyLoaded() {
cdk16625.New().Test()
}


// required to make `go test` recognize the suite.
func TestComplianceSuite(t *testing.T) {
suite.Run(t, new(ComplianceSuite))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package cdk16625

import (
"github.com/aws/jsii/jsii-calc/go/jsiicalc/v3"
abc "github.com/aws/jsii/jsii-calc/go/jsiicalc/v3/cdk16625"
)

func New() abc.Cdk16625 {
c := &cdk16625{}
abc.NewCdk16625_Override(c)
return c
}

type cdk16625 struct{
abc.Cdk16625
}

func (c *cdk16625) Unwrap(rng jsiicalc.IRandomNumberGenerator) *float64 {
return rng.Next()
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import software.amazon.jsii.ReloadingClassLoader;
import software.amazon.jsii.tests.calculator.*;
import software.amazon.jsii.tests.calculator.baseofbase.StaticConsumer;
import software.amazon.jsii.tests.calculator.cdk16625.Cdk16625;
import software.amazon.jsii.tests.calculator.composition.CompositeOperation;
import software.amazon.jsii.tests.calculator.custom_submodule_name.NestingClass.NestedStruct;
import software.amazon.jsii.tests.calculator.lib.EnumFromScopedModule;
Expand Down Expand Up @@ -1799,4 +1800,15 @@ public String repeat(final String word) {

assertEquals(nowAsISO, entropy.increase());
}

@Test
public void classCanBeUsedWhenNotExpressedlyLoaded() {
final Cdk16625 subject = new Cdk16625() {
@Override
protected java.lang.Number unwrap(final IRandomNumberGenerator rng) {
return rng.next();
}
};
subject.test();
}
}
17 changes: 17 additions & 0 deletions packages/@jsii/python-runtime/tests/test_compliance.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
AnonymousImplementationProvider,
UpcasingReflectable,
)
from jsii_calc.cdk16625 import Cdk16625
from jsii_calc.python_self import (
ClassWithSelf,
ClassWithSelfKwarg,
Expand Down Expand Up @@ -1318,3 +1319,19 @@ def repeat(self, word: str) -> str:
entropy = MildEntropy(wall_clock)

assert now == entropy.increase()


def test_class_can_be_used_when_not_expressedly_loaded():
"""
This test verifies that it is possible to receive instances of classes that
belong to submodules that have not been explicitly imported. This implies
the types are registered with the runtime even though the submodule has
never been mentioned in userland.
"""

class Subject(Cdk16625):
def _unwrap(self, rng: IRandomNumberGenerator):
return rng.next()

# This should NOT throw
Subject().test()
22 changes: 22 additions & 0 deletions packages/jsii-calc/lib/cdk16625/donotimport/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import type { IRandomNumberGenerator } from '../../calculator';

/**
* This type demonstrates the ability to receive a callback argument that has a
* type from a submodule not explicitly imported in the user's code. This checks
* that all types available in the assembly can be resolved by the runtime
* library, regardless of whether they were explicitly referenced or not.
*
* @see https://github.com/aws/aws-cdk/issues/16625
*/
export class UnimportedSubmoduleType implements IRandomNumberGenerator {
public constructor(private readonly value: number) {}

/**
* Not quite random, but it'll do.
*
* @returns 1337
*/
public next(): number {
return this.value;
}
}
26 changes: 26 additions & 0 deletions packages/jsii-calc/lib/cdk16625/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import * as assert from 'assert';

import { IRandomNumberGenerator } from '../calculator';
import { UnimportedSubmoduleType } from './donotimport';

export * as donotimport from './donotimport';

export abstract class Cdk16625 {
/**
* Implement this functin to return `gen.next()`. It is extremely important
* that the `donotimport` submodule is NEVER explicitly loaded in the testing
* application (otherwise this test is void).
*
* @param gen a VERY pseudo random number generator.
*/
protected abstract unwrap(gen: IRandomNumberGenerator): number;

/**
* Run this function to verify that everything is working as it should.
*/
public test(): void {
const value = 1337;
const rng = new UnimportedSubmoduleType(value);
assert(this.unwrap(rng) === value);
}
}
2 changes: 2 additions & 0 deletions packages/jsii-calc/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,5 @@ export * as module2702 from './module2702';
export * as module2692 from './module2692';
export * as module2530 from './module2530';
export * as module2700 from './module2700';

export * as cdk16625 from './cdk16625';
135 changes: 134 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,18 @@
"line": 1090
}
},
"jsii-calc.cdk16625": {
"locationInModule": {
"filename": "lib/index.ts",
"line": 23
}
},
"jsii-calc.cdk16625.donotimport": {
"locationInModule": {
"filename": "lib/cdk16625/index.ts",
"line": 6
}
},
"jsii-calc.composition": {
"locationInModule": {
"filename": "lib/calculator.ts",
Expand Down Expand Up @@ -14607,6 +14619,127 @@
],
"symbolId": "lib/compliance:WithPrivatePropertyInConstructor"
},
"jsii-calc.cdk16625.Cdk16625": {
"abstract": true,
"assembly": "jsii-calc",
"docs": {
"stability": "stable"
},
"fqn": "jsii-calc.cdk16625.Cdk16625",
"initializer": {
"docs": {
"stability": "stable"
}
},
"kind": "class",
"locationInModule": {
"filename": "lib/cdk16625/index.ts",
"line": 8
},
"methods": [
{
"docs": {
"stability": "stable",
"summary": "Run this function to verify that everything is working as it should."
},
"locationInModule": {
"filename": "lib/cdk16625/index.ts",
"line": 21
},
"name": "test"
},
{
"abstract": true,
"docs": {
"stability": "stable",
"summary": "Implement this functin to return `gen.next()`. It is extremely important that the `donotimport` submodule is NEVER explicitly loaded in the testing application (otherwise this test is void)."
},
"locationInModule": {
"filename": "lib/cdk16625/index.ts",
"line": 16
},
"name": "unwrap",
"parameters": [
{
"docs": {
"summary": "a VERY pseudo random number generator."
},
"name": "gen",
"type": {
"fqn": "jsii-calc.IRandomNumberGenerator"
}
}
],
"protected": true,
"returns": {
"type": {
"primitive": "number"
}
}
}
],
"name": "Cdk16625",
"namespace": "cdk16625",
"symbolId": "lib/cdk16625/index:Cdk16625"
},
"jsii-calc.cdk16625.donotimport.UnimportedSubmoduleType": {
"assembly": "jsii-calc",
"docs": {
"remarks": "This checks\nthat all types available in the assembly can be resolved by the runtime\nlibrary, regardless of whether they were explicitly referenced or not.",
"see": "https://github.com/aws/aws-cdk/issues/16625",
"stability": "stable",
"summary": "This type demonstrates the ability to receive a callback argument that has a type from a submodule not explicitly imported in the user's code."
},
"fqn": "jsii-calc.cdk16625.donotimport.UnimportedSubmoduleType",
"initializer": {
"docs": {
"stability": "stable"
},
"locationInModule": {
"filename": "lib/cdk16625/donotimport/index.ts",
"line": 12
},
"parameters": [
{
"name": "value",
"type": {
"primitive": "number"
}
}
]
},
"interfaces": [
"jsii-calc.IRandomNumberGenerator"
],
"kind": "class",
"locationInModule": {
"filename": "lib/cdk16625/donotimport/index.ts",
"line": 11
},
"methods": [
{
"docs": {
"returns": "1337",
"stability": "stable",
"summary": "Not quite random, but it'll do."
},
"locationInModule": {
"filename": "lib/cdk16625/donotimport/index.ts",
"line": 19
},
"name": "next",
"overrides": "jsii-calc.IRandomNumberGenerator",
"returns": {
"type": {
"primitive": "number"
}
}
}
],
"name": "UnimportedSubmoduleType",
"namespace": "cdk16625.donotimport",
"symbolId": "lib/cdk16625/donotimport/index:UnimportedSubmoduleType"
},
"jsii-calc.composition.CompositeOperation": {
"abstract": true,
"assembly": "jsii-calc",
Expand Down Expand Up @@ -16646,5 +16779,5 @@
}
},
"version": "3.20.120",
"fingerprint": "f6AcTsdiyd7A1OsnCqScIhOj+U5gmsZC6YnZSsCDh+M="
"fingerprint": "2ST30e+9gb6XYfIMHyGjwObar3fI+/BV0Ex1hIad+2s="
}
Loading

0 comments on commit da55a1e

Please sign in to comment.