-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make REST and GraphQL server running on the same port #1905
Comments
See the discussion in https://github.com/strongloop/loopback-next/pull/1899/files#r227358497 where I am advocating for a new extension package. Cross-posting it here for posterity.
If we keep graphql built into our We have been bitten by this kind of tight coupling many times in LB 3.x. I don't remember a single time when we would wish an independent extension was built into the framework. There were many times we wished a built-in feature was packaged as a standalone extension from the beginning: offline sync, authentication & authorization layer, etc. Also by forcing all Last but not least, while |
The extension package would be living inside the loopback-next as a repo? |
Possibly. Although I think it may be better to keep the code in https://github.com/strongloop/loopback4-extension-graphql, at least until we have our own implementation invoking repositories directly (without the oasgraph layer). |
+1 For @bajtos 's concern. And I would vote for a new extension package too. A good practice for our extensibility. A proposal:
Feel free to correct the details ^ |
Well if there is no REST server, then there is no REST API to create GraphQL interface for. IMO, the component should discover all RestServer instances configured by the app and a create a new GraphQL endpoint for each server instance. In the first iteration, it may be simpler to assume the application is |
Considering the growing importance of GraphQL, I am proposing to make GraphQL a first-class citizen of LB4:
|
Suggestion from @raymondfeng :
|
summary of estimation meeting: See the proposal above ^ let's continue discussion. |
Let's take a look at how oasgraph-cli mounts the GraphQL on express: import * as graphqlHTTP from 'express-graphql'
function startGraphQLServer(oas, port) {
createGraphQlSchema(oas, {
strict: program.strict,
viewer: program.viewer,
addSubOperations: program.addSubOperations,
fillEmptyResponses: program.fillEmptyResponses
})
.then({schema, report}) => {
// ...
app.use('/graphql', graphqlHTTP({
schema: schema,
graphiql: true
}))
// ...
}); Now as far as I understand GraphQL, it uses a single URL with two verbs GET and POST (see https://graphql.org/learn/serving-over-http/). As a result, it should be pretty easy to build a controller that provides two methods (
I think the component should be fairly easy to implement and therefore I prefer to do this properly from the beginning, don't mount the LB4 app on an Express app. |
@bajtos , but internally there is already an express instance used by the API Explorer, I guess?. If you plugin express-graphql, OASGraph and the app.restServer.getApiSpec() you can have the /graphql in the LB4 app. The following is a pseudo code that could be running in the same code where the REST Server is exposing the api explorer (I guess, not sure for now). But at the end, the OASGraph is just transcoding from one format to another, and it is the express-graphql and the express server that will finally be calling the LB4 REST endpoints linked to this schema, right?. const {schema} await OASGraph.createGraphSQSchema(
app.getServer.getApiSpec(),
strict: false,
viewer:true,
addSubOperations: true,
); and then using the internal reference to express (I don't recall very well, but I saw it :-) express.use('/graphql', graphqlHTTP(
{
schema: schema,
graphiql: true
} This express object, can then use the same port as the api explorer?. The blocks are there, but I am not sure if this would be the best approach, but sounds simple . |
Per the early comment by @bajtos:
I think this is arguably more important than just letting oasgraph run on the same port (which on a cursory look should be pretty straightforward, with a similar conclusion to the above comment). Is this within scope of this conversation/does it require a new issue? Is the final step of the current plan referring to creating a new Server implementation or still using oasgraph as a proxy? |
As far as I understand oasgraph, it cannot fetch related models in a single query, it's a limitation given by design. We will have to roll out our own GraphQL layer that will leverage model relation metadata and repository implementations. No need to open a new issue for that, it's already tracked by #656 and possibly by strongloop-archive/loopback4-extension-graphql#4 |
Not at all. REST API Explorer contributes a Controller class implementing dynamic endpoints (e.g. the main HTML page serving the front-end UI) and calls
This code snippet looks reasonable 👍
I prefer to use LB4 handlers instead of accessing underlying Express instance. Here is a code snippet to illustrate what I mean: const urlPath = '/graphql';
const handler = graphqlHTTP({schema, graphiql: true});
const spec = {
'x-visibility': 'undocumented',
responses: {},
};
app.route('get', urlPath, spec, handler);
app.route('post', urlPath, spec, handler); |
I'm excited by this discussion. I've tried implementing the code above on top of express EDITED LINK. Personally, I am really excited to see support for graphql coming along. |
Any updates on this? |
@dougal83, seems like your repo is no longer accessible? I got a 404 when clicking on the link and when looking up for the repo as well. When talking to @bajtos this morning, he also mentioned something similar, i.e. mounting to express app (?). I haven't got a chance to try it out yet, but here is his code snippet:
|
@dhmlau Thanks. I will take a closer look soon. |
Would it be possible to just forward one port to the other? |
This code is OK for me. import {ApiApplication} from './application';
import {ApplicationConfig} from '@loopback/core';
import * as graphqlHTTP from 'express-graphql';
import { createGraphQlSchema } from 'openapi-to-graphql';
import { Oas3 } from 'openapi-to-graphql/lib/types/oas3';
export {ApiApplication};
export async function main(options: ApplicationConfig = {}) {
const app = new ApiApplication(options);
await app.boot();
await app.start();
const url : string = <string>app.restServer.url;
console.log(`Server is running at ${url}`);
const graphqlPath = '/graphql';
const oas: Oas3 = <Oas3>app.restServer.getApiSpec();
const {schema} = await createGraphQlSchema(oas, {
strict: false,
viewer:true,
baseUrl: url,
} );
const handler : graphqlHTTP.Middleware = graphqlHTTP({
schema,
graphiql: true
});
app.mountExpressRouter(graphqlPath, handler );
console.log(`Graphql: ${url}${graphqlPath}`);
return app;
} |
Works for me... until JWT from header is is needed... Its not possible to get the JWT token (valid) from the header and respective service... then get the 401 unauthorised response. also setting the context doesn't help const handler : graphqlHTTP.Middleware = graphqlHTTP( (request, response, graphQLParams) => ({
schema,
pretty: true,
graphiql: true,
context: { request: request, response: response},
})) If I look at the headers in the loopback App context ( found file sequence.ts), each time I make a post or get, then I get 2 different headers... First one has the Auth JWT, Second one does not (seems to override) bit lost now.. |
Managed to get the JWT this way and works fine for me. const graphqlPath = '/graphql'
const oas: Oas3 = <Oas3>app.restServer.getApiSpec()
const {schema} = await createGraphQlSchema(oas, {
strict: false,
viewer: true,
baseUrl: url,
headers: {
'X-Origin': 'GraphQL'
},
tokenJSONpath: '$.jwt'
})
const handler : graphqlHTTP.Middleware = graphqlHTTP( (request, response, graphQLParams) => ({
schema,
pretty: true,
graphiql: process.env.NODE_ENV === 'development', // Needed to activate apollo dev tools (Without true then interface does not load)
context: { jwt: getJwt(request) }
}))
// Get the jwt from the Authorization header and place in context.jwt, which is then referenced in tokenJSONpath
function getJwt(req:any) {
if (req.headers && req.headers.authorization) {
return req.headers.authorization.replace(/^Bearer /, '');
}
}
app.mountExpressRouter(graphqlPath, handler);
console.log(chalk.green(`Graphql API: ${url}${graphqlPath}`)) |
This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the |
This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the |
Description / Steps to reproduce / Feature proposal
Originated from the discussion in #1899 (comment)
Currently the REST server is running at
http://localhost:3000
and usingoasgraph
loosely, the GraphQL server will be running athttp://localhost:3001
.Ideally, it would be REST and GraphQL should share the same port.
Acceptance Criteria (needs discussion/ to be refined)
cc @bajtos @marioestradarosa
The text was updated successfully, but these errors were encountered: