Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed: GToSchema: infinite call: single constructor and recur #37

Merged
merged 5 commits into from
Mar 13, 2022

Conversation

ncaq
Copy link
Contributor

@ncaq ncaq commented Feb 2, 2022

We combined Servant with OpenApi3 and found that the calculation does not finish if there is a cycled single field data type.
Since we were creating data types like the one described in our test data, and it was very annoying that we couldn't create OpenApi data, we investigated the cause and fixed it.
It seems that the test does not pass in the new version of GHC, and the same is true for the existing master, so we left it alone.

Thanks for the great library.
It would be great if you could incorporate it and release it.

why
===

I want to use GHC of supported [haskell/haskell-language-server: Official haskell ide support via language server (LSP). Successor of ghcide & haskell-ide-engine.](https://github.com/haskell/haskell-language-server).
I want timeout for test like below,

~~~hs
-- | create schema of self and ref another.
-- timeout in 1 seconds.
-- because avoid infinite call.
checkToSchemaDeclare :: (HasCallStack, ToSchema a) => Proxy a -> Value -> Spec
checkToSchemaDeclare proxy js = do
  mActuial <- runIO $ timeout (secondsToMicroseconds 1) $ evaluate $ force $ runDeclare (declareSchemaRef proxy) mempty
  case mActuial of
    Nothing -> it "test is timeout" $ mActuial `shouldNotBe` Nothing
    Just actuial -> actuial <=> js

secondsToMicroseconds :: Int -> Int
secondsToMicroseconds seconds = seconds * 10 ^ (6 :: Int)
~~~

However `Schema` is not `NFData`.
So I gave up.
I followed `declareSchemaRef` to make sure that the context of `InsOrdHashMap` disappears.
Since the cause seems to be here, I rewrote it to keep the context and continue the calculation.
@ncaq
Copy link
Contributor Author

ncaq commented Mar 10, 2022

I would like to see this MERGE because it fixes a bug that will be important there in using this library.
Is there something wrong with my PR?
Please point it out and I will fix it.
I don't know who to mentions it to, so I will notify as much as I can.

@maksbotan

@maksbotan
Copy link
Collaborator

Sorry @ncaq! Looks like I had wrong notifications configuration for this repo and did not notice this PR until you pinged me.

@maksbotan maksbotan merged commit f87e8b6 into biocad:master Mar 13, 2022
@ncaq ncaq deleted the fix-single-constructor-recur branch March 14, 2022 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants