GetShopTV / swagger2

Swagger 2.0 data model.
http://hackage.haskell.org/package/swagger2
BSD 3-Clause "New" or "Revised" License
74 stars 59 forks source link

Schema name conflicts #14

Open fizruk opened 9 years ago

fizruk commented 9 years ago

Hypothetical example:

module PhoneBook where

data Contact = Contact
  { name :: String
  , phone :: Integer
  } deriving (Generic, ToSchema)
module MyApp where

import qualified PhoneBook

data Contact = Contact
  { name :: String
  , phoneContact :: PhoneBook.Contact
  , facebookContact :: String
  , googleContact :: String
  } deriving (Generic, ToSchema)

In this example both MyApp.Contact and PhoneBook.Contact have name "Contact" and therefore schema for MyApp.Contact becomes recursive which is wrong.

Perhaps datatypeNameModifier should work with fully qualified names (or it should take module name and datatype name as separate arguments).

fizruk commented 8 years ago

Renaming schemas

I think we can solve this one by introducing helpers for renaming schemas both at the ToSchema level and whole-spec. Here's a sketch:

-- | Rename all schema references using a renaming function.
renameSchemas :: Data s => (Text -> Text) -> s -> s
renameSchemas rename = template %~ renameRef
  where
    renameRef (Ref (Reference name)) = Ref (Reference (rename name))
    renameRef (Inline schema) = Inline (renameSchemas rename schema)

-- | Declare a schema reference with all references renamed using a renaming function.
declareRenamedSchemaRef :: ToSchema a =>
  (Text -> Text) -> proxy a -> Declare Definitions (Referenced Schema)
declareRenamedSchemaRef rename proxy = do
  (defs, schemaRef) <- looks $ runDeclare (declareSchemaRef proxy)
  declare $ HashMap.mapKeys rename defs
  return $ renameSchemas rename schemaRef

Using renaming explicitly to resolve conflicts

Here's how the example above can be fixed with explicit ToSchema instance for MyApp.Contact:

-- MyApp.hs

instance ToSchema Contact where
  declareNamedSchema proxy = do
    phoneBookContactRef <- declareRenamedSchemaRef rename (Proxy :: Proxy PhoneBook.Contact)
    return $ mempty
      & schemaType .~ SwaggerObject
      & schemaProperties .~
        [ ("name", toSchemaRef (Proxy :: Proxy String))
        , ("phoneContact", phoneBookContactRef)
        , ("facebookContact", toSchemaRef (Proxy :: Proxy String))
        , ("googleContact", toSchemaRef (Proxy :: Proxy String)) ]
      & schemaRequired .~ ["name", "phoneContact", "facebookContact", "googleContact"]
    where
      rename "Contact" = "PhoneBookContact"
      rename x = x

Alternatively, for this particular example we can use a shorter definition:

instance ToSchema Contact where
  declareNamedSchema proxy = do
    phoneBookContactRef <- declareRenamedSchemaRef rename (Proxy :: Proxy PhoneBook.Contact)
    return (name, schema
      & schemaProperties . at "phoneContact" ?~ phoneBookContactRef )
    where
      (name, schema) = genericToNamedSchema defaultSchemaOptions proxy
      rename "Contact" = "PhoneBookContact"
      rename x = x

schemaNameModifier option

I think we should add an option to SchemaOptions to allow this kind of renaming:

instance ToSchema Contact where
  declareNamedSchema = genericDeclareNamedSchema defaultSchemaOptions
    { schemaNameModifier = rename }
    where
      rename "Contact" = "PhoneBookContact"
      rename x = x

@dmjio what do you think about this? If it looks fine, should we include these helpers and option in v1.0?

fisx commented 2 years ago

just ran into this problem as well and started thinking about work-arounds. happy to see that's done! :)

ideally, i would like to see a solution that doesn't require the user to think about things (like explicit module paths in the HM keys and in the generated swagger code), and also handles -XPackageImports correctly (or errors out rather than producing wrong results). but i don't think i'll have time for this any time soon.

anyway :+1: on adding this to a release!