Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Added SpringBoot3-GraphQL sample #751

Merged
merged 2 commits into from
Jan 30, 2024
Merged

Conversation

mbfreder
Copy link
Contributor

@mbfreder mbfreder commented Jan 24, 2024

Issue #715:

Description of changes:

Added SpringBoot3-GraphQl sample

Commands to test:

sam build

sam local start-api

curl -X POST http://localhost:3000/graphql -d '{"query":"query petDetails {\n petById(id: \"pet-1\") {\n id\n name\n breed\n owner {\n id\n firstName\n lastName\n }\n }\n}","operationName":"petDetails"}' -H "Content-Type: application/json"

Test results:

{"data":{"petById":{"id":"pet-1","name":"Alpha","breed":"Bulldog","owner":{"id":"owner-1","firstName":"Joshua","lastName":"Bloch"}}}}

By submitting this pull request

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I confirm that I've made a best effort attempt to update all relevant documentation.

@jonife
Copy link
Contributor

jonife commented Jan 24, 2024

Hey Fredric do you mind putting some description on this pr for context?
Add details like test result and commands you ran to test.

@mbfreder
Copy link
Contributor Author

Hey @jonife, I just linked the relevant issue and also added instructions to test. Thanks for the feedback.

@deki deki added this to the Release 2.0 milestone Jan 24, 2024
@@ -0,0 +1,38 @@
# Serverless Spring Boot 3 and Spring-graphQl example
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like how the Spring-graphQl looks.

But I would say either we put

  • just Spring Boot 3 with GraphQL
  • or directly the library name spring-graphql (all lower case)
  • or use the library name, but it seems a little redundant Spring Boot 3 and Spring for GraphQL
  • or maybe Spring-GraphQL if we just want to change casing.

(in order of my personal preference)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, makes sense. I went with the first option.

Comment on lines 8 to 14
import org.springframework.boot.web.servlet.support.SpringBootServletInitializer;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Import;
import org.springframework.web.servlet.HandlerAdapter;
import org.springframework.web.servlet.HandlerExceptionResolver;
import org.springframework.web.servlet.HandlerMapping;
import org.springframework.web.servlet.ModelAndView;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like a few of these imports are not used (SpringBootServletInitializer, HandlerExceptionResolver, ModelAndView)

Try to check for imports in the other files too (I see QueryMapping and SchemaMapping in a different file that are not used there either)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, Thanks.

Handler: com.amazonaws.serverless.sample.springboot3.StreamLambdaHandler::handleRequest
Runtime: java21
CodeUri: .
MemorySize: 1512
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw the other springboot pet-store example already has 1512, but it's a weird number. Normally the memory comes in like powers of 2 (512 is in some other examples). I feel like someone just added a 1 when they wanted more memory to end up with the 1512. But it looks better to use 1024 if that's enough memory (that's what the alt-petstore uses).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@roger-zhangg
Copy link
Member

One suggestion: I think the official name is GraphQL

@mbfreder mbfreder changed the title Added SpringBoot3-GraphQl sample Added SpringBoot3-GraphQL sample Jan 25, 2024
@deki deki merged commit c26036f into aws:main Jan 30, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants