yarax / swagger-to-graphql

Swagger to GraphQL API adapter
MIT License
924 stars 150 forks source link

graphiql generating weird requests #30

Closed jaakko-solita closed 6 years ago

jaakko-solita commented 7 years ago

If I use the following code:

const express = require('express');
const app = express();
var graphqlHTTP = require('express-graphql');
var graphql = require('graphql');
var graphQLSchema = require('swagger-to-graphql');

graphQLSchema('https://rata-beta.digitraffic.fi/swagger/swagger.json').then(schema => {
  app.use('/graphql', graphqlHTTP(() => {
    return {
      schema,
      context: {
        GQLProxyBaseUrl: 'https://rata-beta.digitraffic.fi'
      },
      graphiql: true
    };
  }));

  app.listen(3009, 'localhost', () => {
    console.info(`API is here localhost:3009/graphql`);
  });
}).catch(e => {
  throw e;
});

And then run this query:

{ viewer {  getAllTrainsUsingGET(version:"15223") {
  cancelled
  commuterLineID
  deleted
  departureDate
  operatorShortCode
  operatorUICCode
  runningCurrently
  timetableAcceptanceDate
  timetableType
  trainCategory
  trainNumber
  trainType
  version
} } }

It returns an http error 400 and the requests looks like this: /api/v1/all-trains%7B?version%7D=&version=1523

When it should be like this: api/v1/all-trains?version=1523. The parameters can be found two times in the url along with some url escapes.

I suspect it has something to do with the api having urls like this:

But the all-trains url that I'm testing only has that one optional parameter called version.

Any idea what's wrong?

jaakko-solita commented 7 years ago

It seems the swagger.json has {?version} in the actual path

    "paths": {
        "/api/v1/all-trains{?version}": {
            "get": {
                "tags": [
                    "all-trains"
                ],
                "summary": "Returns trains that are never than {version}",
                "operationId": "getAllTrainsUsingGET",
                "consumes": [
                    "application/json"
                ],
                "produces": [
                    "*/*"
                ],
                "parameters": [{
                        "name": "version",
                        "in": "query",
                        "description": "version",
                        "required": false,
                        "type": "integer",
                        "format": "int64"
                    }
                ],
                "responses": {
                    "200": {
                        "description": "OK",
                        "schema": {
                            "type": "array",
                            "items": {
                                "$ref": "#/definitions/Train"
                            }
                        }
                    }
                }
            }
        },

but Swagger's own "Try it out" feature doesn't mind them: https://rata-beta.digitraffic.fi/swagger-ui.html#!/all-trains/getAllTrainsUsingGET

Consider supporting this URL type?

yarax commented 7 years ago

I'm not sure, that it is a documented feature. According to spec Path templating refers to the usage of curly braces ({}) to mark a section of a URL path as replaceable using path parameters. So curly braces are only for paths. And in your example version is a query parameter, which is pointed as in: query. So I honestly don't get why do you need such syntax