[Feature request]: Add API to change schema ID

[bkoelman]

Is your feature request related to a specific problem? Or an existing feature?

In our codebase, in some scenarios, a constructed System.Type is not sufficient to determine a unique schema ID. To make that fit the existing Swashbuckle SchemaGenerator API, we:

1. Let Swashbuckle generate the schema using SchemaGenerator
2. Rename the generated schema ID, based on an additional string value

The rename happens by obtaining the generated schema from SchemaRepository, removing it and then adding it under a different ID. However, we also need to update the private _reservedIds field for that to work, which we currently do using reflection.

Describe the solution you'd like

I'd like to propose a new method on SchemaRepository to rename the schema ID for an existing System.Type.

public void ReplaceSchemaId(Type oldSchemaType, string newSchemaId);

The implementation would resemble what we currently use:

private static readonly FieldInfo ReservedIdsField; // loaded via reflection

public static void ReplaceSchemaId(this SchemaRepository schemaRepository,
    Type oldSchemaType, string newSchemaId)
{
    ArgumentNullException.ThrowIfNull(schemaRepository);
    ArgumentNullException.ThrowIfNull(oldSchemaType);
    ArgumentException.ThrowIfNullOrEmpty(newSchemaId);

    if (schemaRepository.TryLookupByType(oldSchemaType, out OpenApiSchemaReference? referenceSchema))
    {
        string oldSchemaId = referenceSchema.Reference.Id!;

        IOpenApiSchema targetSchema = schemaRepository.Schemas[oldSchemaId];

        schemaRepository.Schemas.Remove(oldSchemaId);
        schemaRepository.Schemas.Add(newSchemaId, targetSchema);

        var reservedIds = (Dictionary<Type, string>)ReservedIdsField.GetValue(schemaRepository)!;
        reservedIds.Remove(oldSchemaType);
    }
}

Although we don't need it, it may be better to return a boolean indicating whether the replacement has happened.

Once approved, I'm willing to open a PR to implement this.

Additional context

No response

[martincostello]

Seems reasonable - sounds essentially the same as ASP.NET Core's OpenApiOptions.CreateSchemaReferenceId functionality.

[bkoelman]

No, that's the equivalent of https://github.com/domaindrivendev/Swashbuckle.AspNetCore/blob/master/docs/configure-and-customize-swaggergen.md#customize-schema-ids.

Both of those take a Type as input only. In our case, additional information determines the ID. For example, for a model:

public class TodoItem
{
    public Person? Owner { get: set; }
    public Person? Assignee { get: set; }
}

We need to produce a schema per relationship (why is not important here). So we feed Relationship<TodoItem, Person> to the generator, which produces TodoItemPersonRelationship, we rename that to TodoItemOwnerRelationship, then run the generator again and rename the second one to TodoItemAssigneeRelationship.

ASP.NET OpenAPI provides no way to do that.

[martincostello]

Let me know if this changes for this and #3659 are working for you as expected with a prerelease version of 10.1.0, and if all is as expected I'll publish a stable 10.1.0 release.

[bkoelman]

The nullability change does not work (no warning in the else branch):

The rest looks fine, all tests are green.

[martincostello]

I wonder if that's because nullability is still disabled.

[bkoelman]

I wonder if that's because nullability is still disabled.

Yes, #nullable is needed to make it work.

Edit: wrong screenshot.

[martincostello]

I took a quick look at adding #nullable enable just to the SchemaRepository.cs file, but that has a ripple effect into requiring adding nullable annotations for every API listed in PublicAPI.

That's too much work (and churn) for right now.

Rather than re-open #3659, I'll just linked it to the existing issue for enabling nullability at some point.

[martincostello]

I'll look to publish 10.1.0 some time tomorrow, as there's a GitHub incident right now, and I don't want to try and do another release while that's ongoing.

[bkoelman]

I took a quick look at adding #nullable enable just to the SchemaRepository.cs file, but that has a ripple effect into requiring adding nullable annotations for every API listed in PublicAPI.

That's not true. As I mentioned earlier, PublicAPI fully supports mixed codebases where not everything is annotated. Yes, the .txt file changes heavily, but that's not exposed to users. So unless there's an automated process that generates an overview of API changes per release that is published somewhere, I don't see the problem in making the switch.

That's too much work (and churn) for right now.

It's a simple change that can be done in 5 minutes, mostly automated. I can take care of it if you want. I wouldn't worry about churn in a repository where true changes are barely discoverable anyway, because the commit history is flooded with package bumps.

Rather than re-open #3659, I'll just linked it to the existing issue for enabling nullability at some point.

So a new release will be created, listing this issue as fixed, while it wasn't actually fixed? How misleading is that?

Why the urgency to release anyway? The NRT change doesn't even work, and nobody except me cares about this new method.

[martincostello]

Yes, the .txt file changes heavily, but that's not exposed to users

That's what I was referring to, the files, not the code. But I have to change the whole file, and I'm not really enamoured with tackling that work now for the sake of one method.