Add OrdHashMap

[phadej]

I tried with our up, servant-swagger compiled fine; yet the keys seems to be still permuted in UI. Have to investigate still.

[fizruk]

@phadej are the keys permuted in the produced swagger.json or only in the UI?

[phadej]

@fizruk they seem to be permuted in in JSON file already. I actually checked with toSchema on record with ~10 fields).

I added few tests for OrdHashMap. At least mappend preserves the ordering. Dunno how Schema is build though.

I'll check this later today still, comment if you might now the issue from top of your head.

EDIT there is few HashMap here and there, I'll change them first.

[fizruk]

@phadej you did not change HashMap in _swaggerPaths field to OrdHashMap. You also did not change the HashMap in _schemaProperties.

You have only changed Definitions which are used for schema, param, response and security definitions.

[phadej]

Seems that keys are inserted from last to first in generics:

EDIT1

instance (GToSchema f, GToSchema g) => GToSchema (f :*: g) where
  gdeclareNamedSchema opts _ schema = do
    NamedSchema _ gschema <- gdeclareNamedSchema opts (Proxy :: Proxy g) schema
    gdeclareNamedSchema opts (Proxy :: Proxy f) gschema

what is the reason that g is handled first? Is it related to SwaggerMonoid somehow?

EDIT2, if i change the order, then required: ordering changes - but somehow it feels that we should handle products from left-to-right. Current approach feels weird (or there should be a [note]!)

[fizruk]

what is the reason that g is handled first? Is it related to SwaggerMonoid somehow?

I think the main reason is to produce a correct order of fields for non-record product types.
See appendItem.
So if you switch the order, some tests should fail.

Feel free to change the appendItem to put the item to the end of the list.
Alternatively you can inspect Schema in GToSchema (C1 c f) instance and reverse both required and SwaggerItemsArray there.

[phadej]

@fizruk I cleaned up the implementation, and tests are green now. Now the schema building is not-so-performant looking, but is less-quirky, which I think is more important in this domain.

To remember: This is breaking change. Could we merge this but not yet release

• so I (and maybe others) could use bleeding-edge master
• I could wrap up OrdHashMap as separate package, as it seems to be something which could be useful on its own.

EDIT: Todo.Swagger swagger.json is up-to-date test fails in servant-swagger, I suspect ordering issue there, 'cause the lines are of the same length.

[fizruk]

What's this TODO about?

[phadej]

To remember to remove toJSON' - had to add explicit type it to get better error message. It's done now.

[fizruk]

@phadej can you rebase so I can make a clean merge?

Performance is indeed not a big concern for this project.

Yes, I can wait with the release.

OrdHashMap indeed perhaps belongs somewhere else (as does Declare, btw).
Can you create a new repository so that we can discuss that there?

Todo.Swagger swagger.json is up-to-date test fails in servant-swagger

swagger.json should be prettified.
I have just pushed an update in haskell-servant/servant-swagger@baa9b96.
Now you can do this:

$ stack ghci example
>>> writeSwaggerJSON
>>> ^D
$ git diff

You should then see a nice diff.
Ideally, you should see this diff directly in the test suite, but that would require more work.

[phadej]

The properties are now sorted, but I don't know why paths are still unsorted in ui.

[fizruk]

@phadej are paths sorted in JSON?

[phadej]

They are, but unsorted. The issue might be in servant-swagger

[fizruk]

are paths sorted in JSON?

They are, but unsorted.

Sorry, can't parse :)

[phadej]

Ah sorry. paths are unsorted in json, haven't checked the Swagger object though.

[fizruk]

@phadej paths in servant-swagger are collected in HasSwagger instance for :<|>, where two Swagger objects get mappended. So it might have something to do with SwaggerMonoid of OrdHashMap.

[phadej]

Hmm, looks like the Swagger object is fine. I'll recheck again that toEncoding is used, and not toJSON.

[phadej]

Found an issue why paths isn't serialised using toEncoding: because e.g. Swagger toEncoding is default definition which uses toJSON: so order is missed in between.

But to fix that, one would need to do a bit (or maybe quite al lot) of generics work. I'll see if I have the time during the week to try to do it.

[fizruk]

@phadej ah, toJSON messes with the order because it uses HashMap, right?

I guess #59 tries to solve that particular issue?

[phadej]

@fizruk yes. I'm working on OrdHashMap package atm. But will return to this and #59 after that.

[phadej]

@fizruk https://github.com/phadej/insert-ordered-containers

Would be nice if you could review the package, before I release it to Hackage

[phadej]

Finally:

I'd say this is ready for review from my side.

[fizruk]

Definitions is for definitions that are then used with $ref.
Properties are hence not Definitions, but InsOrdHashMap Text.

[phadej]

Changed properties.

[phadej]

ping @fizruk. Rebased the branch.

[fizruk]

@phadej sorry, I didn't have enough time to go through this PR in detail.

I am going to merge this now and apply fixes later if at all.
Are you ok if I wait with the release for a few days?

[phadej]

I'm not in hurry, just checking this isn't forgotten :)

[fizruk]

@phadej it's not forgotten, I had a glance over it before and was going to look more closely later, but got buried in work :)