Fix oneOf/allOf/anyOf and schema module in Discriminator mapping

[lucacorti]

This is the same as #440, with master merged and a fix to allow Schema modules to be used in Discriminator mappings instead of relying on the title:

%Schema{
      type: :object,
      oneOf: [
          MySchema,
          MyOtherSchema
     ],
     discriminator: %Discriminator{
     propertyName: "aprop",
     mapping: %{
      "MY_SCHEMA" => MySchema,
      "MY_OTHER_SCHEMA" => MyOtherSchema
    }
  }
}

Without this casting does not currently work proprerly for schemas with oneOf and Discriminator. It also removes the discriminator propertyName from the path, avoiding it to be added to the JSON pointers in validation errors.

[igormq]

up

[igormq]

hey, someone here willing to merge this pr ?

[mbuhot]

@igormq It's difficult to review and merge this PR with so many commits and changes.
Can the branch be squashed down to a smaller number of commits with clear messages describing each change?

[zorbash]

Missing test.

[lucacorti]

I've not implemented this and am not sure what the functions achieves. @albertored can you take a look?

[zorbash]

LGTM

[lucacorti]

Can this be merged? I've been running this in production for a few months as it fixes a few issues with the existing code where Discriminator is not working. I guess improvements can be done in subsequent PRs.

[mbuhot]

Is Kernel.== necessary here? can it be:

Suggested change

	do: Enum.find(schemas, &Kernel.==(&1."x-struct", discriminator))
	do: Enum.find(schemas, & &1."x-struct" == discriminator)

?

[albertored]

@mbuhot I don't remember if I was the one that originally added this line but I think this was needed to support discriminators where the property is an atom, it is indirectly tested in the test you removed in this same commit.

[mbuhot]

Is there a scenario where the discriminator property is an atom?
The test was casting a map where all keys were already atoms, I'm not sure when that would be required.

[albertored]

I think there are use cases but I don't have a particular one in mind.

My reasoning was that since it is possible to cast an object with atom keys it should be possible also when the discriminator property is an atom

[mbuhot]

@igormq Is there a scenario you have where you need to cast data that already has atom keys?
I'm open to including the functionality, I just want to make sure it's not a hack to fix some other bug we might have :)

024e7d2

[Aduril]

Heho,
I think this still doesn't fix #387 or am I wrong? :-/

The example from the tests still renders a wrong json.

  defmodule AppointmentType do
    require OpenApiSpex

    OpenApiSpex.schema(%{
      title: "AppointmentType",
      type: :object,
      properties: %{
        appointment_type: %Schema{
          type: :string
        }
      },
      required: [:appointment_type]
    })
  end

  defmodule TrainingAppointment do
    OpenApiSpex.schema(%{
      title: "TrainingAppointment",
      description: "Request for a training appointment",
      type: :object,
      allOf: [
        AppointmentType,
        %Schema{
          type: :object,
          properties: %{
            level: %Schema{
              description: "The level of training",
              type: :integer
            }
          },
          required: [:level]
        }
      ]
    })
  end

  defmodule GroomingAppointment do
    OpenApiSpex.schema(%{
      title: "GroomingAppointment",
      description: "Request for a training appointment",
      type: :object,
      allOf: [
        AppointmentType,
        %Schema{
          type: :object,
          properties: %{
            hair_trim: %Schema{
              description: "Whether or not to include a hair trim",
              type: :boolean
            },
            nail_clip: %Schema{
              description: "Whether or not to include nail clip",
              type: :boolean
            }
          },
          required: [:hair_trim, :nail_clip]
        }
      ]
    })
  end

  defmodule PetAppointmentRequest do
    OpenApiSpex.schema(%{
      title: "PetAppointmentRequest",
      description: "POST body for making a pet appointment",
      type: :object,
      oneOf: [
        TrainingAppointment,
        GroomingAppointment
      ],
      discriminator: %OpenApiSpex.Discriminator{
        propertyName: "appointment_type",
        mapping: %{
          "training" => "TrainingAppointment",
          "grooming" => "GroomingAppointment"
        }
      }
    })
  end

[mbuhot]

@Aduril Could you try the latest master branch commit? I've just pushed a change that resolves schema modules appearing in the discriminator mapping. #511

[Aduril]

@mbuhot It works great from my point of view:

Thansk for the quick fix! :)

[mbuhot]

@Aduril Great I'll tag a release 👍