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

Add configurable access for generated clients (#6374) #6416

Merged
merged 5 commits into from
Aug 21, 2023

Conversation

nih0n
Copy link
Contributor

@nih0n nih0n commented Aug 6, 2023

Add an option in graphqlrc.json to specify the access modifier of generated clients, the default access modifier is internal.

Closes #6374

@CLAassistant
Copy link

CLAassistant commented Aug 6, 2023

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added 📚 documentation This issue is about working on our documentation. 🌶️ strawberry shake labels Aug 6, 2023
Copy link
Member

@michaelstaib michaelstaib left a comment

Choose a reason for hiding this comment

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

This looks already quite good!

Can you add a source code generation test like we did here:

You can pass in the generator settings into the assert method ... so that we have the effects of the new modifier under test?

@nih0n nih0n force-pushed the configurable-access branch from 2dd727b to 3a47dde Compare August 9, 2023 08:13
@nih0n
Copy link
Contributor Author

nih0n commented Aug 9, 2023

I added a new test and added a new property to AssertSettings so you can specify the access modifier. Also made some changes in the GetAccessModifier method, so now only public and internal are supported (as private and protected classes doesn't make any sense)

@michaelstaib
Copy link
Member

When looking at the snapshot ... only the client seems to be internal ... models and all other types are still public.

@nih0n nih0n force-pushed the configurable-access branch from 4853411 to d9e5548 Compare August 18, 2023 01:07
@nih0n nih0n marked this pull request as draft August 18, 2023 21:06
@nih0n nih0n marked this pull request as ready for review August 20, 2023 07:58
@github-actions
Copy link

HotChocolate.Execution.Integration.StarWarsCodeFirst.StarWarsCodeFirstTests.GraphQLOrgVariableExample [FAIL]

@sonarcloud
Copy link

sonarcloud bot commented Aug 21, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Patch coverage: 90.00% and no project coverage change.

Comparison is base (eb38b1b) 79.03% compared to head (585500b) 79.04%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6416   +/-   ##
=======================================
  Coverage   79.03%   79.04%           
=======================================
  Files        2911     2911           
  Lines      139246   139246           
=======================================
+ Hits       110051   110061   +10     
+ Misses      29195    29185   -10     
Flag Coverage Δ
unittests 79.04% <90.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...on.CSharp/Generators/RazorSubscriptionGenerator.cs 1.13% <0.00%> (ø)
...Generation.CSharp/Generators/InputTypeGenerator.cs 82.75% <71.42%> (ø)
.../src/CodeGeneration.CSharp/Builders/EnumBuilder.cs 81.25% <77.77%> (ø)
...CodeGeneration.CSharp/Builders/InterfaceBuilder.cs 90.80% <100.00%> (ø)
...ation/src/CodeGeneration.CSharp/CSharpGenerator.cs 66.66% <100.00%> (ø)
...c/CodeGeneration.CSharp/CSharpGeneratorSettings.cs 100.00% <100.00%> (ø)
...odeGeneration.CSharp/Generators/ClientGenerator.cs 100.00% <100.00%> (ø)
...tion.CSharp/Generators/ClientInterfaceGenerator.cs 100.00% <100.00%> (ø)
...eGeneration.CSharp/Generators/DataTypeGenerator.cs 100.00% <100.00%> (ø)
....CSharp/Generators/DependencyInjectionGenerator.cs 99.36% <100.00%> (ø)
... and 20 more

... and 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@michaelstaib michaelstaib merged commit 85a9b27 into ChilliCream:main Aug 21, 2023
@michaelstaib
Copy link
Member

Thank you for contributing ... your PR is merged and we have integrated the changes into this release:
https://github.com/ChilliCream/graphql-platform/releases/tag/13.5.0-preview.24

@nih0n nih0n deleted the configurable-access branch August 22, 2023 01:22
This was referenced Sep 10, 2023
renovate bot referenced this pull request in orso-co/Orso.Arpa.Api Sep 16, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [HotChocolate.Abstractions](https://chillicream.com/)
([source](https://github.com/ChilliCream/graphql-platform)) | nuget |
minor | `13.4.0` -> `13.5.1` |
| [HotChocolate.AspNetCore](https://chillicream.com/)
([source](https://github.com/ChilliCream/graphql-platform)) | nuget |
minor | `13.4.0` -> `13.5.1` |
| [HotChocolate.AspNetCore.Authorization](https://chillicream.com/)
([source](https://github.com/ChilliCream/graphql-platform)) | nuget |
minor | `13.4.0` -> `13.5.1` |
| [HotChocolate.Data](https://chillicream.com/)
([source](https://github.com/ChilliCream/graphql-platform)) | nuget |
minor | `13.4.0` -> `13.5.1` |
| [HotChocolate.Data.EntityFramework](https://chillicream.com/)
([source](https://github.com/ChilliCream/graphql-platform)) | nuget |
minor | `13.4.0` -> `13.5.1` |
| [HotChocolate.Types](https://chillicream.com/)
([source](https://github.com/ChilliCream/graphql-platform)) | nuget |
minor | `13.4.0` -> `13.5.1` |

---

### Release Notes

<details>
<summary>ChilliCream/graphql-platform
(HotChocolate.Abstractions)</summary>

###
[`v13.5.1`](https://github.com/ChilliCream/graphql-platform/releases/tag/13.5.1)

[Compare
Source](https://github.com/ChilliCream/graphql-platform/compare/13.5.0...13.5.1)

##### What's Changed

- StrawberryShake public access modifier must be default. by
[@&#8203;michaelstaib](https://github.com/michaelstaib) in
[https://github.com/ChilliCream/graphql-platform/pull/6472](https://github.com/ChilliCream/graphql-platform/pull/6472)

**Full Changelog**:
ChilliCream/graphql-platform@13.5.0...13.5.1

###
[`v13.5.0`](https://github.com/ChilliCream/graphql-platform/releases/tag/13.5.0)

[Compare
Source](https://github.com/ChilliCream/graphql-platform/compare/13.4.0...13.5.0)

#### What's Changed

- Added GraphQL Request Field Limit. by
[@&#8203;michaelstaib](https://github.com/michaelstaib) in
[https://github.com/ChilliCream/graphql-platform/pull/6381](https://github.com/ChilliCream/graphql-platform/pull/6381)
- Fixed Subscription Complete Issue for ValueType Message. by
[@&#8203;michaelstaib](https://github.com/michaelstaib) in
[https://github.com/ChilliCream/graphql-platform/pull/6383](https://github.com/ChilliCream/graphql-platform/pull/6383)
- Optimized the Type Module Source Generator by
[@&#8203;michaelstaib](https://github.com/michaelstaib) in
[https://github.com/ChilliCream/graphql-platform/pull/6388](https://github.com/ChilliCream/graphql-platform/pull/6388)
- Adds postgres transport for subscriptions by
[@&#8203;PascalSenn](https://github.com/PascalSenn) in
[https://github.com/ChilliCream/graphql-platform/pull/6387](https://github.com/ChilliCream/graphql-platform/pull/6387)
- Expose internals of Fusion to BCP services by
[@&#8203;PascalSenn](https://github.com/PascalSenn) in
[https://github.com/ChilliCream/graphql-platform/pull/6390](https://github.com/ChilliCream/graphql-platform/pull/6390)
- Include .NET 8.0 tools in StrawberryShake package by
[@&#8203;repne](https://github.com/repne) in
[https://github.com/ChilliCream/graphql-platform/pull/6395](https://github.com/ChilliCream/graphql-platform/pull/6395)
- Upgrade default BCP middleware version by
[@&#8203;PascalSenn](https://github.com/PascalSenn) in
[https://github.com/ChilliCream/graphql-platform/pull/6410](https://github.com/ChilliCream/graphql-platform/pull/6410)
- Scope the projection selection properly when using the mutation
conventions. by [@&#8203;hahn-kev](https://github.com/hahn-kev) in
[https://github.com/ChilliCream/graphql-platform/pull/6444](https://github.com/ChilliCream/graphql-platform/pull/6444)
- Reintroduce RequestContextAccessor by
[@&#8203;michaelstaib](https://github.com/michaelstaib) in
[https://github.com/ChilliCream/graphql-platform/pull/6454](https://github.com/ChilliCream/graphql-platform/pull/6454)
- Publicly expose ability to register an error type on a mutation field
by [@&#8203;benmccallum](https://github.com/benmccallum) in
[https://github.com/ChilliCream/graphql-platform/pull/6463](https://github.com/ChilliCream/graphql-platform/pull/6463)
- Removed Path Pooling by
[@&#8203;michaelstaib](https://github.com/michaelstaib) in
[https://github.com/ChilliCream/graphql-platform/pull/6394](https://github.com/ChilliCream/graphql-platform/pull/6394)
- Fixed naming issues with generic types by
[@&#8203;N-Olbert](https://github.com/N-Olbert) in
[https://github.com/ChilliCream/graphql-platform/pull/6461](https://github.com/ChilliCream/graphql-platform/pull/6461)
- Fixed Batch Pool for DataLoader was cleared to early. by
[@&#8203;michaelstaib](https://github.com/michaelstaib) in
[https://github.com/ChilliCream/graphql-platform/pull/6465](https://github.com/ChilliCream/graphql-platform/pull/6465)
- Added configurable access for generated Strawberry Shake clients
([#&#8203;6374](https://github.com/ChilliCream/graphql-platform/issues/6374))
by [@&#8203;nih0n](https://github.com/nih0n) in
[https://github.com/ChilliCream/graphql-platform/pull/6416](https://github.com/ChilliCream/graphql-platform/pull/6416)

**Full Changelog**:
ChilliCream/graphql-platform@13.4.0...13.5.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 10pm every weekday,every
weekend,before 5am every weekday" in timezone Europe/Berlin, Automerge -
At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/orso-co/Orso.Arpa.Api).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi44My4wIiwidXBkYXRlZEluVmVyIjoiMzYuODMuMCIsInRhcmdldEJyYW5jaCI6ImRldmVsb3AifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 documentation This issue is about working on our documentation. 🌶️ strawberry shake
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strawberry Shake: Configurable access for generated clients
3 participants