Accidentally truncating fields generates invalid Swagger specifications

[bitemyapp]

https://github.com/bitemyapp/swagger2-empty-field-name-bug/blob/master/app/Main.hs

Prelude> main
{
    "minItems": 2,
    "items": [
        {
            "type": "string"
        },
        {
            "maximum": 9223372036854775807,
            "minimum": -9223372036854775808,
            "type": "integer"
        }
    ],
    "maxItems": 2,
    "type": "array"
}
{
    "required": [
        "Blah",
        "Woot"
    ],
    "type": "object",
    "properties": {
        "Blah": {
            "type": "string"
        },
        "Woot": {
            "maximum": 9223372036854775807,
            "minimum": -9223372036854775808,
            "type": "integer"
        }
    }
}

Now, I realize dropping too many characters is a user error, but there's absolutely no warning anywhere that you made a mistake and it generates Swagger that is rejected by editor.swagger.io

The error you get from the editor is: items must be an object

It seems like it's handling an empty string poorly somewhere and it tricks it into thinking the object is an array when it should just blow up with an error. I don't entirely understand the Generics code so I cannot say for sure.

Another source of confusion is that it flattens nested types out in a way that makes a deeply nested type with this mistake bubble up to the root type.

[fizruk]

It seems like it's handling an empty string poorly somewhere and it tricks it into thinking the object is an array when it should just blow up with an error.

Right, swagger2 turns records with empty field names (which is what we get from GHC.Generics for non-record product types) intro tuples represented by JSON arrays. This is the code responsible:

swagger2/src/Data/Swagger/Internal/Schema.hs

Lines 784 to 800 in eaf9fc9

	withFieldSchema opts _ isRequiredField schema = do
	ref <- gdeclareSchemaRef opts (Proxy :: Proxy f)
	return $
	if T.null fname
	then schema
	& type_ .~ SwaggerArray
	& items %~ appendItem ref
	& maxItems %~ Just . maybe 1 (+1) -- increment maxItems
	& minItems %~ Just . maybe 1 (+1) -- increment minItems
	else schema
	& type_ .~ SwaggerObject
	& properties . at fname ?~ ref
	& if isRequiredField
	then required %~ (++ [fname])
	else id
	where
	fname = T.pack (fieldLabelModifier opts (selName (Proxy3 :: Proxy3 s f p)))

You can see that is checks T.null fname to decide whether it will continue building a SwaggerObject or a SwaggerArray.

We could insert a check in there to see that it was fieldNameModifier that made is empty and issue a warning or and error.

Unfortunately there is no gentle way to handle warnings/errors in swagger2, which makes our choices here quite limited. There's only one place where we use error in Generic code (which should be impossible):

swagger2/src/Data/Swagger/Internal/Schema.hs

Line 780 in eaf9fc9

appendItem _ _ = error "GToSchema.appendItem: cannot append to SwaggerItemsObject"

Other errors are TypeErrors and non-termination (e.g. for recursive definitions as in #183).

We could also expand Definitions to include warnings/errors so that you would be able to look at those for the entire Swagger document you build. But then users will have to know about it somehow.

@bitemyapp what would you expect to happen in your case?

Another source of confusion is that it flattens nested types out in a way that makes a deeply nested type with this mistake bubble up to the root type.

Can you provide an example?