Rewrite `GraphQL.Analysis`
### Motivation
While we strive to write clear code, we have historically struggled at Hasura with having very different styles and standards across the codebase. There's been efforts to standardize our coding style, we have an official styleguide that isn't maintained as closely as it should... We still have some work in front of us.
However, in the last ~year or so, there's been a huge push towards incrementally improving the situation. As part of this we've been blocking PRs that don't add enough comments, or don't improve the files that they touch.
While looking at `Hasura.GraphQL.Analyse`, it became apparent that this file did not meet the engineering standards that I would expect to see addressed during a code review. Some ways in which I think it falls short:
- lack of documentation
- no clear distinction between public / internal components
- "unidiomatic" Haskell code (such as using `Either Result Error`)
While there's no problem with a file looking like this during development, those issues should have been caught at review time. The fact that they weren't indicates a problem in our process that we will need to address: code quality and maintainability is paramount, and we all need to do our part.
### Description
This PR rewrites all of `Hasura.GraphQL.Analyze`, and adapts `Hasura.Server.OpenAPI` accordingly where needed. I've attempted to clarify names and add documentation based on my understanding of the code, and to clean what was unused (such as field variables). I don't think this PR is good enough as is, and I welcome criticism where I got my comments wrong / am happy to help y'all add more.
This PR makes one small change in the way error messages are reported (and adjusts the corresponding test accordingly); each error message is now prefixed with the path within the selection set:
```
⚠️ $.test.foo.bar.baz.mizpelled: field 'mizpelled' not found in object 'Baz'
```
### Note
This PR is currently **on top of #3962**. You can preview the changes in isolation by [diffing the branches](https://github.com/hasura/graphql-engine-mono/compare/nicuveo/clean-rest-endpoint-inconsistency-check..nicuveo/rewrite-analysis).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3963
Co-authored-by: paritosh-08 <85472423+paritosh-08@users.noreply.github.com>
GitOrigin-RevId: 5ec38e0e753f0c12096a350db0737658495e2f15
2022-04-04 08:53:59 +03:00
|
|
|
{-# LANGUAGE ViewPatterns #-}
|
|
|
|
-- This prevents hlint errors on the "pattern" lens.
|
|
|
|
{-# LANGUAGE NoPatternSynonyms #-}
|
|
|
|
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
module Hasura.Server.OpenAPI (buildOpenAPI) where
|
2021-10-06 10:15:14 +03:00
|
|
|
|
|
|
|
import Control.Lens
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
import Control.Monad.Circular
|
2021-12-22 11:30:15 +03:00
|
|
|
import Data.Aeson qualified as J
|
|
|
|
import Data.HashMap.Strict qualified as Map
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
import Data.HashMap.Strict.InsOrd.Extended qualified as OMap
|
2022-03-01 19:03:23 +03:00
|
|
|
import Data.HashMap.Strict.Multi qualified as MMap
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
import Data.Monoid (Any (..))
|
2021-10-06 10:15:14 +03:00
|
|
|
import Data.OpenApi
|
|
|
|
import Data.OpenApi.Declare
|
|
|
|
import Data.Text qualified as T
|
2021-12-22 11:30:15 +03:00
|
|
|
import Data.Text.NonEmpty
|
2022-03-01 19:03:23 +03:00
|
|
|
import Data.Trie qualified as Trie
|
2022-05-04 13:56:54 +03:00
|
|
|
import Hasura.Base.Error
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
import Hasura.Base.Instances ()
|
2022-03-08 12:48:21 +03:00
|
|
|
import Hasura.GraphQL.Analyse
|
2022-05-04 13:56:54 +03:00
|
|
|
import Hasura.GraphQL.Transport.HTTP.Protocol
|
2021-10-06 10:15:14 +03:00
|
|
|
import Hasura.Prelude hiding (get, put)
|
|
|
|
import Hasura.RQL.Types.Endpoint
|
|
|
|
import Hasura.RQL.Types.QueryCollection
|
Rewrite `GraphQL.Analysis`
### Motivation
While we strive to write clear code, we have historically struggled at Hasura with having very different styles and standards across the codebase. There's been efforts to standardize our coding style, we have an official styleguide that isn't maintained as closely as it should... We still have some work in front of us.
However, in the last ~year or so, there's been a huge push towards incrementally improving the situation. As part of this we've been blocking PRs that don't add enough comments, or don't improve the files that they touch.
While looking at `Hasura.GraphQL.Analyse`, it became apparent that this file did not meet the engineering standards that I would expect to see addressed during a code review. Some ways in which I think it falls short:
- lack of documentation
- no clear distinction between public / internal components
- "unidiomatic" Haskell code (such as using `Either Result Error`)
While there's no problem with a file looking like this during development, those issues should have been caught at review time. The fact that they weren't indicates a problem in our process that we will need to address: code quality and maintainability is paramount, and we all need to do our part.
### Description
This PR rewrites all of `Hasura.GraphQL.Analyze`, and adapts `Hasura.Server.OpenAPI` accordingly where needed. I've attempted to clarify names and add documentation based on my understanding of the code, and to clean what was unused (such as field variables). I don't think this PR is good enough as is, and I welcome criticism where I got my comments wrong / am happy to help y'all add more.
This PR makes one small change in the way error messages are reported (and adjusts the corresponding test accordingly); each error message is now prefixed with the path within the selection set:
```
⚠️ $.test.foo.bar.baz.mizpelled: field 'mizpelled' not found in object 'Baz'
```
### Note
This PR is currently **on top of #3962**. You can preview the changes in isolation by [diffing the branches](https://github.com/hasura/graphql-engine-mono/compare/nicuveo/clean-rest-endpoint-inconsistency-check..nicuveo/rewrite-analysis).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3963
Co-authored-by: paritosh-08 <85472423+paritosh-08@users.noreply.github.com>
GitOrigin-RevId: 5ec38e0e753f0c12096a350db0737658495e2f15
2022-04-04 08:53:59 +03:00
|
|
|
import Hasura.RQL.Types.SchemaCache hiding (FieldInfo)
|
2021-10-06 10:15:14 +03:00
|
|
|
import Language.GraphQL.Draft.Syntax qualified as G
|
2021-12-22 11:30:15 +03:00
|
|
|
import Network.HTTP.Media.MediaType ((//))
|
2021-10-06 10:15:14 +03:00
|
|
|
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
--------------------------------------------------------------------------------
|
|
|
|
-- API
|
|
|
|
|
|
|
|
buildOpenAPI :: (MonadError QErr m, MonadFix m) => SchemaCache -> m OpenApi
|
|
|
|
buildOpenAPI schemaCache = do
|
|
|
|
(defs, spec) <- flip runDeclareT mempty do
|
|
|
|
endpoints <- buildAllEndpoints schemaCache (scAdminIntrospection schemaCache)
|
|
|
|
pure $
|
|
|
|
mempty
|
|
|
|
& paths .~ fmap fst endpoints
|
|
|
|
& info . title .~ "Rest Endpoints"
|
|
|
|
& info . description
|
|
|
|
?~ "This OpenAPI specification is automatically generated by Hasura." <> foldMap snd endpoints
|
|
|
|
pure $ spec & components . schemas .~ defs
|
|
|
|
|
|
|
|
--------------------------------------------------------------------------------
|
|
|
|
-- Endpoint
|
|
|
|
|
|
|
|
buildAllEndpoints ::
|
|
|
|
(MonadError QErr m, MonadFix m) =>
|
|
|
|
SchemaCache ->
|
|
|
|
G.SchemaIntrospection ->
|
|
|
|
DeclareM m (InsOrdHashMap String (PathItem, Text))
|
|
|
|
buildAllEndpoints schemaCache schemaTypes =
|
|
|
|
foldl' (OMap.unionWith (<>)) mempty <$> sequence do
|
|
|
|
-- for each path in the trie of endpoints
|
|
|
|
endpointMap <- Trie.elems $ scEndpoints schemaCache
|
|
|
|
-- for each method at that path
|
|
|
|
(method, metadataList) <- MMap.toList endpointMap
|
|
|
|
-- for each metadata associated with that method
|
|
|
|
metadata <- metadataList
|
|
|
|
-- build the corresponding path item and list of messages
|
|
|
|
pure $ buildEndpoint schemaTypes method metadata
|
|
|
|
|
|
|
|
buildEndpoint ::
|
|
|
|
(MonadError QErr m, MonadFix m) =>
|
|
|
|
G.SchemaIntrospection ->
|
|
|
|
EndpointMethod ->
|
|
|
|
EndpointMetadata GQLQueryWithText ->
|
|
|
|
DeclareM m (InsOrdHashMap String (PathItem, Text))
|
|
|
|
buildEndpoint schemaTypes method EndpointMetadata {..} = do
|
|
|
|
let -- extracting endpoint info
|
|
|
|
GQLQueryWithText (queryText, GQLQuery queryDocument) = _edQuery _ceDefinition
|
|
|
|
singleOperation <- lift $ getSingleOperation (GQLReq Nothing (GQLExecDoc (G.getExecutableDefinitions queryDocument)) Nothing)
|
|
|
|
let (fromMaybe (Structure mempty mempty) -> analysis, messages) = analyzeGraphQLQuery schemaTypes singleOperation
|
|
|
|
|
|
|
|
-- extracting endpoint url and name
|
|
|
|
pathComponents = splitPath formatVariable id _ceUrl
|
|
|
|
-- TODO: why are we doing this? we are dropping references to variables IIUC?
|
|
|
|
formatVariable variable = "{" <> T.drop 1 variable <> "}"
|
|
|
|
endpointURL = "/api/rest/" <> T.intercalate "/" pathComponents
|
|
|
|
|
|
|
|
-- building endpoint properties
|
|
|
|
endpointVarList = collectParams analysis _ceUrl
|
|
|
|
endpointDescription =
|
|
|
|
fold _ceComment
|
|
|
|
<> "***\nThe GraphQl query for this endpoint is:\n``` graphql\n"
|
|
|
|
<> queryText
|
|
|
|
<> "\n```"
|
|
|
|
endpointName = unNonEmptyText $ unEndpointName _ceName
|
|
|
|
reqBody <- buildRequestBody analysis
|
|
|
|
response <- buildResponse analysis method endpointURL
|
|
|
|
|
|
|
|
let -- building the PathItem
|
|
|
|
operation =
|
|
|
|
mempty
|
|
|
|
& description ?~ endpointDescription
|
|
|
|
& summary ?~ endpointName
|
|
|
|
& parameters .~ (Inline xHasuraAdminSecret : endpointVarList)
|
|
|
|
& requestBody .~ reqBody
|
|
|
|
& responses .~ Responses Nothing (OMap.singleton 200 $ Inline response)
|
|
|
|
pathItem =
|
|
|
|
mempty & case method of
|
|
|
|
GET -> get ?~ operation
|
|
|
|
PUT -> put ?~ operation
|
|
|
|
POST -> post ?~ operation
|
|
|
|
PATCH -> patch ?~ operation
|
|
|
|
DELETE -> delete ?~ operation
|
|
|
|
|
|
|
|
-- making summary of errors
|
|
|
|
formattedMessages =
|
|
|
|
if null messages
|
|
|
|
then ""
|
|
|
|
else "\n\nEndpoint \"" <> endpointName <> "\":" <> foldMap ("\n- ⚠️ " <>) messages
|
|
|
|
|
|
|
|
pure $ OMap.singleton (T.unpack endpointURL) (pathItem, formattedMessages)
|
|
|
|
|
|
|
|
--------------------------------------------------------------------------------
|
|
|
|
-- Parameters
|
|
|
|
|
|
|
|
-- | Given the 'Structure' of a query, generate the corresponding parameters.
|
|
|
|
--
|
|
|
|
-- We expect one optional parameter per known scalar variable.
|
|
|
|
collectParams :: Structure -> EndpointUrl -> [Referenced Param]
|
|
|
|
collectParams (Structure _ vars) eURL = do
|
|
|
|
(G.unName -> varName, VariableInfo {..}) <- Map.toList vars
|
|
|
|
case _viTypeInfo of
|
|
|
|
-- we do not allow input objects or enums in parameters
|
|
|
|
InputFieldObjectInfo _ -> empty
|
|
|
|
InputFieldEnumInfo _ -> empty
|
|
|
|
InputFieldScalarInfo _ -> case _viType of
|
|
|
|
-- we do not allow arrays in parameters
|
|
|
|
G.TypeList _ _ -> empty
|
|
|
|
G.TypeNamed nullability typeName -> case getReferenceScalarInfo typeName of
|
|
|
|
-- we do not allow unknown scalars in parameters
|
|
|
|
Nothing -> empty
|
|
|
|
Just (refType, typePattern, _shouldInline) -> do
|
|
|
|
-- TODO: there's duplication between this piece of the code and the request body
|
|
|
|
-- do we want to ensure consistency by deduplicating?
|
|
|
|
let isRequired = not $ G.unNullability nullability || isJust _viDefaultValue
|
|
|
|
desc =
|
|
|
|
if isRequired
|
|
|
|
then Just $ "_\"" <> varName <> "\" is required (enter it either in parameters or request body)_"
|
|
|
|
else Nothing
|
|
|
|
-- TODO: document this
|
|
|
|
-- NOTE: URL Variable name ':' prefix is removed for `elem` lookup.
|
|
|
|
pathVars = map (T.drop 1) $ concat $ splitPath pure (const []) eURL
|
|
|
|
pure $
|
|
|
|
-- We always inline the schema, since we might need to add the default value.
|
|
|
|
Inline $
|
|
|
|
mempty
|
|
|
|
& name .~ varName
|
|
|
|
& description .~ desc
|
|
|
|
& in_ .~ (if varName `elem` pathVars then ParamPath else ParamQuery)
|
|
|
|
& schema
|
|
|
|
?~ Inline
|
|
|
|
( mempty
|
|
|
|
& default_ .~ (gqlToJsonValue <$> _viDefaultValue)
|
|
|
|
& type_ ?~ refType
|
|
|
|
& pattern .~ typePattern
|
|
|
|
)
|
|
|
|
|
|
|
|
--------------------------------------------------------------------------------
|
|
|
|
-- Request body
|
|
|
|
|
|
|
|
-- | Given the 'Structure' of a query, generate the corresponding 'RequestBody'.
|
|
|
|
--
|
|
|
|
-- We always expect an object that has a field per variable of the query if
|
|
|
|
-- there is at least one variable in the query; otherwise we don't expect a
|
|
|
|
-- request body.
|
|
|
|
buildRequestBody ::
|
|
|
|
(MonadError QErr m, MonadFix m) =>
|
|
|
|
Structure ->
|
|
|
|
DeclareM m (Maybe (Referenced RequestBody))
|
|
|
|
buildRequestBody Structure {..} = do
|
|
|
|
let vars = Map.toList _stVariables
|
|
|
|
if null vars
|
|
|
|
then pure Nothing
|
|
|
|
else do
|
|
|
|
(varProperties, Any isBodyRequired) <-
|
|
|
|
runCircularT $
|
|
|
|
mconcat <$> for vars \(varName, varInfo) -> do
|
|
|
|
(resolvedVarInfo, isVarRequired) <- buildVariableSchema varInfo
|
|
|
|
pure (OMap.singleton (G.unName varName) resolvedVarInfo, Any isVarRequired)
|
|
|
|
pure $
|
|
|
|
Just $
|
|
|
|
Inline $
|
|
|
|
mempty
|
|
|
|
& description ?~ "Query parameters can also be provided in the request body as a JSON object"
|
|
|
|
& required ?~ isBodyRequired
|
|
|
|
& content
|
|
|
|
.~ OMap.singleton
|
|
|
|
("application" // "json")
|
|
|
|
( mempty
|
|
|
|
& schema
|
|
|
|
?~ Inline
|
|
|
|
( mempty
|
|
|
|
& type_ ?~ OpenApiObject
|
|
|
|
& properties .~ varProperties
|
|
|
|
)
|
|
|
|
)
|
|
|
|
|
|
|
|
-- | Given the information about a variable, build the corresponding schema.
|
|
|
|
--
|
|
|
|
-- Returns the generated schema, and a boolean indicating whether the variable
|
|
|
|
-- is required.
|
|
|
|
buildVariableSchema ::
|
|
|
|
(MonadError QErr m, MonadFix m) =>
|
|
|
|
VariableInfo ->
|
|
|
|
CircularT (G.Name, G.Nullability) (Referenced Schema) (DeclareM m) (Referenced Schema, Bool)
|
|
|
|
buildVariableSchema VariableInfo {..} = do
|
|
|
|
-- a variable is optional if:
|
|
|
|
-- - it has a default value
|
|
|
|
-- - it's nullable
|
|
|
|
-- - it's a known scalar (it will be available as a parameter)
|
|
|
|
let hasDefaultValue = isJust _viDefaultValue
|
|
|
|
isNullable = G.isNullable _viType
|
|
|
|
isKnownScalar = case _viType of
|
|
|
|
G.TypeNamed _ typeName -> isJust (getReferenceScalarInfo typeName)
|
|
|
|
_ -> False
|
|
|
|
isOptional = hasDefaultValue || isNullable || isKnownScalar
|
|
|
|
|
|
|
|
baseSchema <- buildInputFieldSchema _viType _viTypeInfo
|
|
|
|
varSchema <- case _viDefaultValue of
|
|
|
|
-- If we don't need to modify the schema by adding a default value, we leave
|
|
|
|
-- it unchanged (which means it might be a reference rather than inlined).
|
|
|
|
Nothing -> pure baseSchema
|
|
|
|
-- If we need to modify it, then we might have to dereference it.
|
|
|
|
Just defaultValue -> do
|
|
|
|
varSchema <- case baseSchema of
|
|
|
|
Inline varSchema -> pure varSchema
|
|
|
|
Ref (Reference refName) -> do
|
|
|
|
-- We introspect the declarations to retrieve the underlying
|
|
|
|
-- schema. we know the type will have a corresponding declaration
|
|
|
|
-- since all references are created by 'declareType'. This might
|
|
|
|
-- result in an unnecessary component declaration if here is the only
|
|
|
|
-- place the reference would have been used.
|
|
|
|
declarations <- lift look
|
|
|
|
OMap.lookup refName declarations
|
|
|
|
-- DeclareT doesn't have a MonadError instance, hence the need for
|
|
|
|
-- explicit lifting.
|
|
|
|
`onNothing` lift (lift $ throw500 "internal error: declareType returned an invalid reference")
|
|
|
|
pure $ Inline $ varSchema & default_ ?~ gqlToJsonValue defaultValue
|
|
|
|
|
|
|
|
pure (varSchema, not isOptional)
|
|
|
|
|
|
|
|
-- | Given the information about an input type, build the corresponding schema.
|
|
|
|
buildInputFieldSchema ::
|
|
|
|
MonadFix m =>
|
|
|
|
G.GType ->
|
|
|
|
InputFieldInfo ->
|
|
|
|
CircularT (G.Name, G.Nullability) (Referenced Schema) (DeclareM m) (Referenced Schema)
|
|
|
|
buildInputFieldSchema gType = \case
|
|
|
|
-- this input field is a scalar: we attempt to declare it
|
|
|
|
InputFieldScalarInfo scalarInfo ->
|
|
|
|
lift $ applyModifiers gType $ buildScalarSchema scalarInfo
|
|
|
|
-- this input field is an enum: we declare it
|
|
|
|
InputFieldEnumInfo enumInfo ->
|
|
|
|
lift $ applyModifiers gType $ buildEnumSchema enumInfo
|
|
|
|
-- this input field is an object: we declare it
|
|
|
|
InputFieldObjectInfo InputObjectInfo {..} ->
|
|
|
|
applyModifiers gType \typeName nullability -> withCircular (typeName, nullability) do
|
|
|
|
fields <-
|
|
|
|
for (Map.toList _ioiFields) \(fieldName, (fieldType, fieldTypeInfo)) -> do
|
|
|
|
fieldSchema <- buildInputFieldSchema fieldType fieldTypeInfo
|
|
|
|
pure (G.unName fieldName, fieldSchema)
|
|
|
|
let objectSchema =
|
|
|
|
mempty
|
|
|
|
& title ?~ G.unName typeName
|
|
|
|
& description .~ fmap G.unDescription (G._iotdDescription _ioiTypeDefinition)
|
|
|
|
& properties .~ OMap.fromList fields
|
|
|
|
& type_ ?~ OpenApiObject
|
|
|
|
& nullable ?~ G.unNullability nullability
|
|
|
|
lift $ declareType typeName nullability objectSchema
|
|
|
|
|
|
|
|
--------------------------------------------------------------------------------
|
|
|
|
-- Response
|
|
|
|
|
|
|
|
-- | Given the 'Structure' of a query, generate the corresponding 'Response'.
|
|
|
|
buildResponse ::
|
|
|
|
Monad m =>
|
Rewrite `GraphQL.Analysis`
### Motivation
While we strive to write clear code, we have historically struggled at Hasura with having very different styles and standards across the codebase. There's been efforts to standardize our coding style, we have an official styleguide that isn't maintained as closely as it should... We still have some work in front of us.
However, in the last ~year or so, there's been a huge push towards incrementally improving the situation. As part of this we've been blocking PRs that don't add enough comments, or don't improve the files that they touch.
While looking at `Hasura.GraphQL.Analyse`, it became apparent that this file did not meet the engineering standards that I would expect to see addressed during a code review. Some ways in which I think it falls short:
- lack of documentation
- no clear distinction between public / internal components
- "unidiomatic" Haskell code (such as using `Either Result Error`)
While there's no problem with a file looking like this during development, those issues should have been caught at review time. The fact that they weren't indicates a problem in our process that we will need to address: code quality and maintainability is paramount, and we all need to do our part.
### Description
This PR rewrites all of `Hasura.GraphQL.Analyze`, and adapts `Hasura.Server.OpenAPI` accordingly where needed. I've attempted to clarify names and add documentation based on my understanding of the code, and to clean what was unused (such as field variables). I don't think this PR is good enough as is, and I welcome criticism where I got my comments wrong / am happy to help y'all add more.
This PR makes one small change in the way error messages are reported (and adjusts the corresponding test accordingly); each error message is now prefixed with the path within the selection set:
```
⚠️ $.test.foo.bar.baz.mizpelled: field 'mizpelled' not found in object 'Baz'
```
### Note
This PR is currently **on top of #3962**. You can preview the changes in isolation by [diffing the branches](https://github.com/hasura/graphql-engine-mono/compare/nicuveo/clean-rest-endpoint-inconsistency-check..nicuveo/rewrite-analysis).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3963
Co-authored-by: paritosh-08 <85472423+paritosh-08@users.noreply.github.com>
GitOrigin-RevId: 5ec38e0e753f0c12096a350db0737658495e2f15
2022-04-04 08:53:59 +03:00
|
|
|
Structure ->
|
|
|
|
EndpointMethod ->
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
Text ->
|
|
|
|
DeclareM m Response
|
|
|
|
buildResponse (Structure fields _) endpointMethod endpointURL = do
|
|
|
|
fs <- buildSelectionSchema $ Map.toList fields
|
2021-12-22 11:30:15 +03:00
|
|
|
pure $
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
mempty
|
|
|
|
& content .~ OMap.singleton ("application" // "json") (mempty & schema ?~ Inline fs)
|
|
|
|
& description .~ "Responses for " <> tshow endpointMethod <> " " <> endpointURL
|
2021-12-22 11:30:15 +03:00
|
|
|
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
-- | Given a list of fields and their types, build a corresponding schema.
|
|
|
|
buildSelectionSchema ::
|
|
|
|
Monad m =>
|
Rewrite `GraphQL.Analysis`
### Motivation
While we strive to write clear code, we have historically struggled at Hasura with having very different styles and standards across the codebase. There's been efforts to standardize our coding style, we have an official styleguide that isn't maintained as closely as it should... We still have some work in front of us.
However, in the last ~year or so, there's been a huge push towards incrementally improving the situation. As part of this we've been blocking PRs that don't add enough comments, or don't improve the files that they touch.
While looking at `Hasura.GraphQL.Analyse`, it became apparent that this file did not meet the engineering standards that I would expect to see addressed during a code review. Some ways in which I think it falls short:
- lack of documentation
- no clear distinction between public / internal components
- "unidiomatic" Haskell code (such as using `Either Result Error`)
While there's no problem with a file looking like this during development, those issues should have been caught at review time. The fact that they weren't indicates a problem in our process that we will need to address: code quality and maintainability is paramount, and we all need to do our part.
### Description
This PR rewrites all of `Hasura.GraphQL.Analyze`, and adapts `Hasura.Server.OpenAPI` accordingly where needed. I've attempted to clarify names and add documentation based on my understanding of the code, and to clean what was unused (such as field variables). I don't think this PR is good enough as is, and I welcome criticism where I got my comments wrong / am happy to help y'all add more.
This PR makes one small change in the way error messages are reported (and adjusts the corresponding test accordingly); each error message is now prefixed with the path within the selection set:
```
⚠️ $.test.foo.bar.baz.mizpelled: field 'mizpelled' not found in object 'Baz'
```
### Note
This PR is currently **on top of #3962**. You can preview the changes in isolation by [diffing the branches](https://github.com/hasura/graphql-engine-mono/compare/nicuveo/clean-rest-endpoint-inconsistency-check..nicuveo/rewrite-analysis).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3963
Co-authored-by: paritosh-08 <85472423+paritosh-08@users.noreply.github.com>
GitOrigin-RevId: 5ec38e0e753f0c12096a350db0737658495e2f15
2022-04-04 08:53:59 +03:00
|
|
|
[(G.Name, FieldInfo)] ->
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
DeclareM m Schema
|
|
|
|
buildSelectionSchema fields = do
|
Rewrite `GraphQL.Analysis`
### Motivation
While we strive to write clear code, we have historically struggled at Hasura with having very different styles and standards across the codebase. There's been efforts to standardize our coding style, we have an official styleguide that isn't maintained as closely as it should... We still have some work in front of us.
However, in the last ~year or so, there's been a huge push towards incrementally improving the situation. As part of this we've been blocking PRs that don't add enough comments, or don't improve the files that they touch.
While looking at `Hasura.GraphQL.Analyse`, it became apparent that this file did not meet the engineering standards that I would expect to see addressed during a code review. Some ways in which I think it falls short:
- lack of documentation
- no clear distinction between public / internal components
- "unidiomatic" Haskell code (such as using `Either Result Error`)
While there's no problem with a file looking like this during development, those issues should have been caught at review time. The fact that they weren't indicates a problem in our process that we will need to address: code quality and maintainability is paramount, and we all need to do our part.
### Description
This PR rewrites all of `Hasura.GraphQL.Analyze`, and adapts `Hasura.Server.OpenAPI` accordingly where needed. I've attempted to clarify names and add documentation based on my understanding of the code, and to clean what was unused (such as field variables). I don't think this PR is good enough as is, and I welcome criticism where I got my comments wrong / am happy to help y'all add more.
This PR makes one small change in the way error messages are reported (and adjusts the corresponding test accordingly); each error message is now prefixed with the path within the selection set:
```
⚠️ $.test.foo.bar.baz.mizpelled: field 'mizpelled' not found in object 'Baz'
```
### Note
This PR is currently **on top of #3962**. You can preview the changes in isolation by [diffing the branches](https://github.com/hasura/graphql-engine-mono/compare/nicuveo/clean-rest-endpoint-inconsistency-check..nicuveo/rewrite-analysis).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3963
Co-authored-by: paritosh-08 <85472423+paritosh-08@users.noreply.github.com>
GitOrigin-RevId: 5ec38e0e753f0c12096a350db0737658495e2f15
2022-04-04 08:53:59 +03:00
|
|
|
props <- for fields \(fieldName, fieldInfo) -> do
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
fieldSchema <- buildFieldSchema fieldInfo
|
Rewrite `GraphQL.Analysis`
### Motivation
While we strive to write clear code, we have historically struggled at Hasura with having very different styles and standards across the codebase. There's been efforts to standardize our coding style, we have an official styleguide that isn't maintained as closely as it should... We still have some work in front of us.
However, in the last ~year or so, there's been a huge push towards incrementally improving the situation. As part of this we've been blocking PRs that don't add enough comments, or don't improve the files that they touch.
While looking at `Hasura.GraphQL.Analyse`, it became apparent that this file did not meet the engineering standards that I would expect to see addressed during a code review. Some ways in which I think it falls short:
- lack of documentation
- no clear distinction between public / internal components
- "unidiomatic" Haskell code (such as using `Either Result Error`)
While there's no problem with a file looking like this during development, those issues should have been caught at review time. The fact that they weren't indicates a problem in our process that we will need to address: code quality and maintainability is paramount, and we all need to do our part.
### Description
This PR rewrites all of `Hasura.GraphQL.Analyze`, and adapts `Hasura.Server.OpenAPI` accordingly where needed. I've attempted to clarify names and add documentation based on my understanding of the code, and to clean what was unused (such as field variables). I don't think this PR is good enough as is, and I welcome criticism where I got my comments wrong / am happy to help y'all add more.
This PR makes one small change in the way error messages are reported (and adjusts the corresponding test accordingly); each error message is now prefixed with the path within the selection set:
```
⚠️ $.test.foo.bar.baz.mizpelled: field 'mizpelled' not found in object 'Baz'
```
### Note
This PR is currently **on top of #3962**. You can preview the changes in isolation by [diffing the branches](https://github.com/hasura/graphql-engine-mono/compare/nicuveo/clean-rest-endpoint-inconsistency-check..nicuveo/rewrite-analysis).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3963
Co-authored-by: paritosh-08 <85472423+paritosh-08@users.noreply.github.com>
GitOrigin-RevId: 5ec38e0e753f0c12096a350db0737658495e2f15
2022-04-04 08:53:59 +03:00
|
|
|
pure (G.unName fieldName, fieldSchema)
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
pure $ mempty & properties .~ OMap.fromList props
|
2021-12-22 11:30:15 +03:00
|
|
|
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
-- | Build the schema for a given output type.
|
|
|
|
buildFieldSchema ::
|
|
|
|
Monad m =>
|
Rewrite `GraphQL.Analysis`
### Motivation
While we strive to write clear code, we have historically struggled at Hasura with having very different styles and standards across the codebase. There's been efforts to standardize our coding style, we have an official styleguide that isn't maintained as closely as it should... We still have some work in front of us.
However, in the last ~year or so, there's been a huge push towards incrementally improving the situation. As part of this we've been blocking PRs that don't add enough comments, or don't improve the files that they touch.
While looking at `Hasura.GraphQL.Analyse`, it became apparent that this file did not meet the engineering standards that I would expect to see addressed during a code review. Some ways in which I think it falls short:
- lack of documentation
- no clear distinction between public / internal components
- "unidiomatic" Haskell code (such as using `Either Result Error`)
While there's no problem with a file looking like this during development, those issues should have been caught at review time. The fact that they weren't indicates a problem in our process that we will need to address: code quality and maintainability is paramount, and we all need to do our part.
### Description
This PR rewrites all of `Hasura.GraphQL.Analyze`, and adapts `Hasura.Server.OpenAPI` accordingly where needed. I've attempted to clarify names and add documentation based on my understanding of the code, and to clean what was unused (such as field variables). I don't think this PR is good enough as is, and I welcome criticism where I got my comments wrong / am happy to help y'all add more.
This PR makes one small change in the way error messages are reported (and adjusts the corresponding test accordingly); each error message is now prefixed with the path within the selection set:
```
⚠️ $.test.foo.bar.baz.mizpelled: field 'mizpelled' not found in object 'Baz'
```
### Note
This PR is currently **on top of #3962**. You can preview the changes in isolation by [diffing the branches](https://github.com/hasura/graphql-engine-mono/compare/nicuveo/clean-rest-endpoint-inconsistency-check..nicuveo/rewrite-analysis).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3963
Co-authored-by: paritosh-08 <85472423+paritosh-08@users.noreply.github.com>
GitOrigin-RevId: 5ec38e0e753f0c12096a350db0737658495e2f15
2022-04-04 08:53:59 +03:00
|
|
|
FieldInfo ->
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
DeclareM m (Referenced Schema)
|
|
|
|
buildFieldSchema = \case
|
|
|
|
-- this output field is a scalar: we attempt to declare it
|
|
|
|
FieldScalarInfo gType scalarInfo -> applyModifiers gType $ buildScalarSchema scalarInfo
|
|
|
|
-- this output field is an enum: we declare it
|
|
|
|
FieldEnumInfo gType scalarInfo -> applyModifiers gType $ buildEnumSchema scalarInfo
|
|
|
|
-- this output field is an object: we inline it
|
|
|
|
FieldObjectInfo gType ObjectInfo {..} -> applyModifiers gType $ \typeName nullability -> do
|
|
|
|
objectSchema <- buildSelectionSchema $ Map.toList _oiSelection
|
|
|
|
pure $
|
|
|
|
Inline $
|
|
|
|
objectSchema
|
Rewrite `GraphQL.Analysis`
### Motivation
While we strive to write clear code, we have historically struggled at Hasura with having very different styles and standards across the codebase. There's been efforts to standardize our coding style, we have an official styleguide that isn't maintained as closely as it should... We still have some work in front of us.
However, in the last ~year or so, there's been a huge push towards incrementally improving the situation. As part of this we've been blocking PRs that don't add enough comments, or don't improve the files that they touch.
While looking at `Hasura.GraphQL.Analyse`, it became apparent that this file did not meet the engineering standards that I would expect to see addressed during a code review. Some ways in which I think it falls short:
- lack of documentation
- no clear distinction between public / internal components
- "unidiomatic" Haskell code (such as using `Either Result Error`)
While there's no problem with a file looking like this during development, those issues should have been caught at review time. The fact that they weren't indicates a problem in our process that we will need to address: code quality and maintainability is paramount, and we all need to do our part.
### Description
This PR rewrites all of `Hasura.GraphQL.Analyze`, and adapts `Hasura.Server.OpenAPI` accordingly where needed. I've attempted to clarify names and add documentation based on my understanding of the code, and to clean what was unused (such as field variables). I don't think this PR is good enough as is, and I welcome criticism where I got my comments wrong / am happy to help y'all add more.
This PR makes one small change in the way error messages are reported (and adjusts the corresponding test accordingly); each error message is now prefixed with the path within the selection set:
```
⚠️ $.test.foo.bar.baz.mizpelled: field 'mizpelled' not found in object 'Baz'
```
### Note
This PR is currently **on top of #3962**. You can preview the changes in isolation by [diffing the branches](https://github.com/hasura/graphql-engine-mono/compare/nicuveo/clean-rest-endpoint-inconsistency-check..nicuveo/rewrite-analysis).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3963
Co-authored-by: paritosh-08 <85472423+paritosh-08@users.noreply.github.com>
GitOrigin-RevId: 5ec38e0e753f0c12096a350db0737658495e2f15
2022-04-04 08:53:59 +03:00
|
|
|
& title ?~ G.unName typeName
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
& description .~ fmap G.unDescription (G._otdDescription _oiTypeDefinition)
|
|
|
|
& type_ ?~ OpenApiObject
|
|
|
|
& nullable ?~ G.unNullability nullability
|
|
|
|
|
|
|
|
--------------------------------------------------------------------------------
|
|
|
|
-- Scalars
|
|
|
|
|
|
|
|
-- | Craft the OpenAPI 'Schema' for a given scalar. Any non-standard scalar will
|
|
|
|
-- instead be declared, and returned by reference.
|
|
|
|
buildScalarSchema ::
|
|
|
|
Monad m =>
|
|
|
|
ScalarInfo ->
|
|
|
|
G.Name ->
|
|
|
|
G.Nullability ->
|
|
|
|
DeclareM m (Referenced Schema)
|
|
|
|
buildScalarSchema ScalarInfo {..} scalarName nullability = do
|
|
|
|
case getReferenceScalarInfo scalarName of
|
|
|
|
-- there is an existing OpenAPI scalar we can map this to: we inline if we can
|
|
|
|
Just (refType, refPattern, shouldInline) -> do
|
|
|
|
let resultSchema =
|
|
|
|
baseSchema
|
|
|
|
& type_ ?~ refType
|
|
|
|
& pattern .~ refPattern
|
|
|
|
if shouldInline
|
|
|
|
then pure $ Inline resultSchema
|
|
|
|
else declareType scalarName nullability resultSchema
|
|
|
|
-- there isn't: we declare that type and return a reference to it
|
|
|
|
Nothing ->
|
|
|
|
declareType scalarName nullability $
|
|
|
|
baseSchema
|
|
|
|
& description .~ fmap G.unDescription (G._stdDescription _siTypeDefinition)
|
2021-10-06 10:15:14 +03:00
|
|
|
where
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
baseSchema =
|
|
|
|
mempty
|
|
|
|
& title ?~ G.unName scalarName
|
|
|
|
& nullable ?~ G.unNullability nullability
|
|
|
|
|
|
|
|
-- | Retrieve info associated with a given scalar, if it can be mapped to a
|
|
|
|
-- built-in OpenAPI scalar. On a match, we return a tuple indiciating which
|
|
|
|
-- scalar should be used, a pattern, and a boolean indicating whether this type
|
|
|
|
-- should be inlined.
|
|
|
|
getReferenceScalarInfo :: G.Name -> Maybe (OpenApiType, Maybe Pattern, Bool)
|
|
|
|
getReferenceScalarInfo =
|
|
|
|
G.unName >>> T.toLower >>> \case
|
|
|
|
"int" -> Just (OpenApiInteger, Nothing, True)
|
|
|
|
"float" -> Just (OpenApiNumber, Nothing, True)
|
|
|
|
"double" -> Just (OpenApiNumber, Nothing, True)
|
|
|
|
"uuid" -> Just (OpenApiString, Just "[a-f0-9]{8}-[a-f0-9]{4}-4[a-f0-9]{3}-[89aAbB][a-f0-9]{3}-[a-f0-9]{12}", False)
|
|
|
|
"bool" -> Just (OpenApiBoolean, Nothing, True)
|
|
|
|
"boolean" -> Just (OpenApiBoolean, Nothing, True)
|
|
|
|
"string" -> Just (OpenApiString, Nothing, True)
|
|
|
|
"id" -> Just (OpenApiString, Nothing, True)
|
|
|
|
_ -> Nothing
|
|
|
|
|
|
|
|
--------------------------------------------------------------------------------
|
|
|
|
-- Enums
|
|
|
|
|
|
|
|
-- | Craft the OpenAPI 'Schema' for a given enum.
|
|
|
|
buildEnumSchema ::
|
|
|
|
Monad m =>
|
|
|
|
EnumInfo ->
|
|
|
|
G.Name ->
|
|
|
|
G.Nullability ->
|
|
|
|
DeclareM m (Referenced Schema)
|
|
|
|
buildEnumSchema EnumInfo {..} enumName nullability =
|
|
|
|
declareType enumName nullability $
|
|
|
|
mempty
|
|
|
|
& title ?~ G.unName enumName
|
|
|
|
& enum_ ?~ enumValues
|
|
|
|
& nullable ?~ G.unNullability nullability
|
|
|
|
& description .~ fmap G.unDescription (G._etdDescription _eiTypeDefinition)
|
2021-12-22 11:30:15 +03:00
|
|
|
where
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
enumValues :: [J.Value]
|
|
|
|
enumValues =
|
|
|
|
G._etdValueDefinitions _eiTypeDefinition <&> \G.EnumValueDefinition {..} ->
|
|
|
|
J.String $ G.unName $ G.unEnumValue _evdName
|
|
|
|
|
|
|
|
--------------------------------------------------------------------------------
|
|
|
|
-- Declaring GraphQL types
|
|
|
|
|
|
|
|
-- | Given an annotated GraphQL type (such as @[[Foo!]]!@ and a callback
|
|
|
|
-- function to be used on the actual underlying type, construct a 'Schema' by
|
|
|
|
-- recursively applying modifiers.
|
|
|
|
applyModifiers ::
|
|
|
|
Monad m =>
|
Rewrite `GraphQL.Analysis`
### Motivation
While we strive to write clear code, we have historically struggled at Hasura with having very different styles and standards across the codebase. There's been efforts to standardize our coding style, we have an official styleguide that isn't maintained as closely as it should... We still have some work in front of us.
However, in the last ~year or so, there's been a huge push towards incrementally improving the situation. As part of this we've been blocking PRs that don't add enough comments, or don't improve the files that they touch.
While looking at `Hasura.GraphQL.Analyse`, it became apparent that this file did not meet the engineering standards that I would expect to see addressed during a code review. Some ways in which I think it falls short:
- lack of documentation
- no clear distinction between public / internal components
- "unidiomatic" Haskell code (such as using `Either Result Error`)
While there's no problem with a file looking like this during development, those issues should have been caught at review time. The fact that they weren't indicates a problem in our process that we will need to address: code quality and maintainability is paramount, and we all need to do our part.
### Description
This PR rewrites all of `Hasura.GraphQL.Analyze`, and adapts `Hasura.Server.OpenAPI` accordingly where needed. I've attempted to clarify names and add documentation based on my understanding of the code, and to clean what was unused (such as field variables). I don't think this PR is good enough as is, and I welcome criticism where I got my comments wrong / am happy to help y'all add more.
This PR makes one small change in the way error messages are reported (and adjusts the corresponding test accordingly); each error message is now prefixed with the path within the selection set:
```
⚠️ $.test.foo.bar.baz.mizpelled: field 'mizpelled' not found in object 'Baz'
```
### Note
This PR is currently **on top of #3962**. You can preview the changes in isolation by [diffing the branches](https://github.com/hasura/graphql-engine-mono/compare/nicuveo/clean-rest-endpoint-inconsistency-check..nicuveo/rewrite-analysis).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3963
Co-authored-by: paritosh-08 <85472423+paritosh-08@users.noreply.github.com>
GitOrigin-RevId: 5ec38e0e753f0c12096a350db0737658495e2f15
2022-04-04 08:53:59 +03:00
|
|
|
G.GType ->
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
(G.Name -> G.Nullability -> m (Referenced Schema)) ->
|
|
|
|
m (Referenced Schema)
|
|
|
|
applyModifiers gtype fun = case gtype of
|
|
|
|
G.TypeNamed nullability typeName -> fun typeName nullability
|
|
|
|
G.TypeList nullability innerType -> do
|
|
|
|
s <- applyModifiers innerType fun
|
2021-12-22 11:30:15 +03:00
|
|
|
pure $
|
|
|
|
Inline $
|
|
|
|
mempty
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
& nullable ?~ G.unNullability nullability
|
2021-12-22 11:30:15 +03:00
|
|
|
& type_ ?~ OpenApiArray
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
& items ?~ OpenApiItemsObject s
|
2021-12-22 11:30:15 +03:00
|
|
|
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
-- | Adds a declaration for the given type, returns a schema that references it.
|
|
|
|
declareType :: Monad m => G.Name -> G.Nullability -> Schema -> DeclareM m (Referenced Schema)
|
|
|
|
declareType typeName nullability s = do
|
|
|
|
let refName = mkReferenceName typeName nullability
|
|
|
|
declare $ OMap.singleton refName s
|
|
|
|
pure $ Ref $ Reference refName
|
2022-01-21 08:39:08 +03:00
|
|
|
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
-- | Crafts a reference name for a given type.
|
|
|
|
--
|
|
|
|
-- We use the fact that JSON references allow characters that GraphQL types
|
|
|
|
-- don't: we make a different reference for non-nullable type by using the
|
|
|
|
-- GraphQL convention of suffixing the name by @!@.
|
|
|
|
--
|
|
|
|
-- See Note [Nullable types in OpenAPI].
|
|
|
|
mkReferenceName :: G.Name -> G.Nullability -> Text
|
|
|
|
mkReferenceName (G.unName -> typeName) (G.Nullability isNullable) =
|
|
|
|
if isNullable
|
|
|
|
then typeName
|
|
|
|
else typeName <> "!"
|
|
|
|
|
|
|
|
--------------------------------------------------------------------------------
|
|
|
|
-- Local helpers
|
|
|
|
|
|
|
|
type DeclareM = DeclareT (Definitions Schema)
|
|
|
|
|
|
|
|
-- | Variable definition for x-hasura-admin-secret
|
|
|
|
xHasuraAdminSecret :: Param
|
|
|
|
xHasuraAdminSecret =
|
2021-10-06 10:15:14 +03:00
|
|
|
mempty
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
& name .~ "x-hasura-admin-secret"
|
|
|
|
& description ?~ "Your x-hasura-admin-secret will be used for authentication of the API request."
|
|
|
|
& in_ .~ ParamHeader
|
|
|
|
& schema ?~ Inline (mempty & type_ ?~ OpenApiString)
|
2021-10-06 10:15:14 +03:00
|
|
|
|
Rewrite OpenAPI
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
2022-06-30 15:55:56 +03:00
|
|
|
-- | Convert a GraphQL value to an equivalent JSON representation.
|
|
|
|
--
|
|
|
|
-- TODO: can we deduplicate this?
|
2021-12-22 11:30:15 +03:00
|
|
|
gqlToJsonValue :: G.Value Void -> J.Value
|
|
|
|
gqlToJsonValue = \case
|
|
|
|
G.VNull -> J.Null
|
|
|
|
G.VInt n -> J.toJSON n
|
|
|
|
G.VFloat sci -> J.toJSON sci
|
|
|
|
G.VString txt -> J.toJSON txt
|
|
|
|
G.VBoolean b -> J.toJSON b
|
|
|
|
G.VEnum ev -> J.toJSON ev
|
|
|
|
G.VList lst -> J.toJSON $ gqlToJsonValue <$> lst
|
|
|
|
G.VObject obj -> J.toJSON $ gqlToJsonValue <$> obj
|