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

Validation issue with nestedProperties and member elision. #2294

Open
libre-man opened this issue May 21, 2024 · 7 comments
Open

Validation issue with nestedProperties and member elision. #2294

libre-man opened this issue May 21, 2024 · 7 comments
Labels
needs-reproduction This issue needs reproduction.

Comments

@libre-man
Copy link

I have a resource like this:

string TenantId

resource TenantResource {
    identifiers: { tenantId: TenantId }
    read: GetTenant
}

And the following models:

@output
structure GetTenantResponse {
    @required
    @nestedProperties
    @httpPayload
    tenant: Tenant
}

structure Tenant for TenantResource {
    $tenantId
}

When compiling this I get: Member tenantId does not target a property or identifier for resource TenantResource.

When I remove the nestedProperties trait I don't get this error anymore.

@libre-man
Copy link
Author

If I add @resourceIdentifier("tenantId") to $tenantId it does work.

@hpmellema
Copy link
Contributor

I suspect the root issue here is that the $tenantId field is missing a @required trait. Elided resource Identifiers should always be required fields. The error messages returned for elided identifiers are currently a bit unclear and probably need to be updated.

@libre-man
Copy link
Author

Even adding a required trait does not help unfortunately.

@natehardison
Copy link

Same issue -- adding @required does not help, but adding @resourceIdentifier does

@sugmanue
Copy link
Contributor

Thomas and Nate

Care to provide a self-contained minimal model we can use to reproduce the issue?

I just tried with this and validates just fine:

$version: "2"

namespace com.example

string TenantId

resource TenantResource {
    identifiers: { tenantId: TenantId }
    read: GetTenant
}

@readonly
operation GetTenant {
    input: GetTenantRequest
    output: GetTenantResponse
}

@input
structure GetTenantRequest for TenantResource {
    @required
    $tenantId
}

@output
structure GetTenantResponse {
    @required
    @nestedProperties
    @httpPayload
    tenant: Tenant
}

structure Tenant for TenantResource {
    $tenantId
}

@sugmanue sugmanue added the needs-reproduction This issue needs reproduction. label May 23, 2024
@natehardison
Copy link

natehardison commented May 24, 2024

Sure, building off of @libre-man's starting point:

$version: "2"

namespace com.example

string TenantId

resource TenantResource {
    identifiers: { tenantId: TenantId }
    properties: { name: String }
    read: GetTenant
}

@readonly
operation GetTenant {
    input := for TenantResource {
        @required
        $tenantId
    }

    output := for TenantResource {
        @required
        @nestedProperties
        data: Tenant
    }
}


structure Tenant for TenantResource {
    $tenantId
    $name
}

example validation fail:

smithy validate example.smithy

──  ERROR  ──────────────────────────────────────── ResourceOperationInputOutput
Shape: com.example#Tenant$tenantId
File:  example.smithy:29:5

28| structure Tenant for TenantResource {
29|     $tenantId
  |     ^

Member tenantId does not target a property or identifier for resource
com.example#TenantResource

FAILURE: Validated 226 shapes (ERROR: 1)

Notably, if I remove the properties field entirely from TenantResource, then things validate just fine.

Am using Smithy version 1.49.0.

@medwards
Copy link

medwards commented Sep 2, 2024

I don't think this is the same as libre-man's model. If you remove nestedProperties you get

Member data does not target a property or identifier for resource com.example#TenantResource

but "When I remove the nestedProperties trait I don't get this error anymore."

I found this issue because of the "Member data does not target" error. It may not be the same root cause as libre-man but I found closely reading Resource property binding validation to be very helpful in figuring out what was wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-reproduction This issue needs reproduction.
Projects
None yet
Development

No branches or pull requests

5 participants