Model assembler claims conflicting members - false positive

[kubukoz]

Consider the following model IDL:

$version: "2"
namespace test

@mixin
structure Common {
  @required description: String
}

structure Str with [Common] {
  description: String = "test"
}

When you load and serialize this to Smithy again, you'll get an apply node for the @default trait:

$version: "2.0"

namespace test

@mixin
structure Common {
    @required
    description: String
}

structure Str with [Common] {}

apply Str$description @default("test")

This is also reflected in the normal ModelSerializer (outputting JSON):

JSON dump

{
    "smithy": "2.0",
    "shapes": {
        "test#Common": {
            "type": "structure",
            "members": {
                "description": {
                    "target": "smithy.api#String",
                    "traits": {
                        "smithy.api#required": {}
                    }
                }
            },
            "traits": {
                "smithy.api#mixin": {}
            }
        },
        "test#Str": {
            "type": "structure",
            "mixins": [
                {
                    "target": "test#Common"
                }
            ],
            "members": {}
        },
        "test#Str$description": {
            "type": "apply",
            "traits": {
                "smithy.api#default": "test"
            }
        }
    }
}

and it also loads fine if you have this one model file.

However, if you have another model file that happens to duplicate these structures, and you try to load them both during assembly, it fails:

software.amazon.smithy.model.validation.ValidatedResultException: Result contained ERROR severity validation events: 
[ERROR] test#Str: Conflicting shape definition for `test#Str` found at `test2.json [18, 21]` and `test.json [18, 21]`. Members differ: [description] vs [description] | Model test.json:18:21
	at software.amazon.smithy.model.validation.ValidatedResult.validate(ValidatedResult.java:145)
	at software.amazon.smithy.model.validation.ValidatedResult.unwrap(ValidatedResult.java:132)

That seems wrong: the model merging process is able to resolve the "conflict" once, but not twice, in what otherwise seemed like an idempotent process.

Is this something that should be fixed?

The entire process can be reproduced with the following scala-cli script:

//> using scala "3.3.1"
//> using option "-Wunused:imports"
//> using lib "software.amazon.smithy:smithy-model:1.39.1"
import software.amazon.smithy.model.shapes.SmithyIdlModelSerializer
import software.amazon.smithy.model.node.Node
import software.amazon.smithy.model.shapes.ModelSerializer
import software.amazon.smithy.model.loader.ModelAssembler
import software.amazon.smithy.model.Model
import scala.jdk.CollectionConverters._

val model =
  """$version: "2"
    |namespace test
    |
    |@mixin
    |structure Common {
    |  @required description: String
    |}
    |
    |structure Str with [Common] {
    |  description: String = "test"
    |}
    |""".stripMargin

val loaded = Model
  .assembler()
  .putProperty(ModelAssembler.DISABLE_JAR_CACHE, true)
  .addUnparsedModel(
    "test.smithy",
    model,
  )
  .assemble()
  .unwrap()

SmithyIdlModelSerializer.builder().build().serialize(loaded).asScala.head._2

val serialized = Node.prettyPrintJson(ModelSerializer.builder().build().serialize(loaded))

Model
  .assembler()
  .addUnparsedModel("test.json", serialized)
  .addUnparsedModel("test2.json", serialized)
  .assemble()
  .unwrap()

[kubukoz]

Hey team, this is causing us more trouble nowadays. Can someone please triage the issue and drop some hints for a solution so that we can contribute a fix?

[haydenbaker]

The diff between the shape is due to the required trait from the mixin only being applied once since the trait gets “claimed” the first time the shape is built and removed from the unclaimed traits. When the shape gets built for the second time, the unclaimed traits is empty, so the member doesn’t get the required trait applied, and hence the comparison between previous and built shows a diff.

There's a few ways we could go about this, but a simple way is to track what is "claimed" in addition to "unclaimed" (and don't remove items from unclaimed), and then if anything in unclaimed is not in claimed, validation event should be emitted (in emitUnclaimedTraits).