christianhelle / refitter

A tool for generating Refit interfaces and contracts from OpenAPI specifications
https://refitter.github.io
MIT License
187 stars 41 forks source link

Generating IObservable type response #322

Closed janfolbrecht closed 7 months ago

janfolbrecht commented 7 months ago

Refit supports also IObservable reactive response type in interfaces so I added this feature to Refitter because it was missing. I added property ReturnIApiResponse to Settings and RefitGeneratorSettings It replaces System.Threading.Task import by System.Reactive and it generates IObservable instead of Task and IObservable<> instead of Task<>. I added one unit test to test generated output is compilable. I had to add Unit class manually to mock missing System.Reactive.Unit class.

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.33%. Comparing base (67e06eb) to head (0b3e451). Report is 37 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #322 +/- ## ======================================= Coverage 97.33% 97.33% ======================================= Files 63 63 Lines 2398 2402 +4 ======================================= + Hits 2334 2338 +4 Misses 40 40 Partials 24 24 ``` | [Flag](https://app.codecov.io/gh/christianhelle/refitter/pull/322/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Christian+Helle) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/christianhelle/refitter/pull/322/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Christian+Helle) | `97.33% <100.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Christian+Helle#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

christianhelle commented 7 months ago

@all-contributors please add @janfolbrecht for ideas and code

allcontributors[bot] commented 7 months ago

@christianhelle

I've put up a pull request to add @janfolbrecht! :tada:

janfolbrecht commented 7 months ago

Hi, yes System.Reactive is for Unit class. It is reactive "void" replacement. I did not use HttpResponseMessage return type with refit yet, so I don't know if it works as replacement of void or not. Jan

st 21. 2. 2024 v 22:08 odesílatel Christian Helle @.***> napsal:

@.**** commented on this pull request.

In src/Refitter.Core/RefitInterfaceImports.cs https://github.com/christianhelle/refitter/pull/322#discussion_r1498309910 :

  • {
  • "Refit",
  • "System.Collections.Generic",
  • "System.Text.Json.Serialization",
  • };
  • public static string[] GetImportedNamespaces(RefitGeneratorSettings settings)
  • {
  • var namespaces = new List(defaultNamespases);
  • if (settings.UseCancellationTokens)
  • {
  • namespaces.Add("System.Threading");
  • }
  • if (settings.ReturnIObservable)
  • {
  • namespaces.Add("System.Reactive");

I'm not sure I fully understand why the generated code needs a dependency on System.Reactive. What do you need the Unit type for?

In src/Refitter.Core/RefitInterfaceGenerator.cs https://github.com/christianhelle/refitter/pull/322#discussion_r1498311172 :

 }
  • private string GetAsyncOperationType(bool withVoidReturnType) =>
  • settings.ReturnIObservable
  • ? "IObservable" + (withVoidReturnType ? "" : string.Empty)

I don't have much experience working with IObservable but why do we need Unit? What does it do and what is it for?

The examples in the Refit README https://github.com/reactiveui/refit?tab=readme-ov-file#retrieving-the-response uses an HttpResponseMessage together with IObservable

// Returns the raw response, as an IObservable that can be used with the// Reactive Extensions[Get("/users/{user}")]IObservable GetUser(string user);

— Reply to this email directly, view it on GitHub https://github.com/christianhelle/refitter/pull/322#pullrequestreview-1894410924, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKB3O3AGWAJGG2TR4VONFS3YUZO3ZAVCNFSM6AAAAABDTUFMDGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQOJUGQYTAOJSGQ . You are receiving this because you were mentioned.Message ID: @.***>

christianhelle commented 7 months ago

@janfolbrecht are you up for fixing the things I suggested? I perfectly understand if don't feel like it and since the requested changes are trivial I can easily do this myself so we can get this merged

janfolbrecht commented 7 months ago

Hi, I will try to fix it. I am not used to working with GitHub, so it is new.

čt 22. 2. 2024 v 14:31 odesílatel Christian Helle @.***> napsal:

@janfolbrecht https://github.com/janfolbrecht are you up for fixing the things I suggested? I perfectly understand if don't feel like it and since the requested changes are trivial I can easily do this myself so we can get this merged

— Reply to this email directly, view it on GitHub https://github.com/christianhelle/refitter/pull/322#issuecomment-1959460310, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKB3O3HED4XQLH2W4RVQC6DYU5CEJAVCNFSM6AAAAABDTUFMDGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNJZGQ3DAMZRGA . You are receiving this because you were mentioned.Message ID: @.***>

janfolbrecht commented 7 months ago

Thank you. If you can make suggested changes please do it. Jan

sonarcloud[bot] commented 7 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

christianhelle commented 7 months ago

@janfolbrecht this contribution is now released to nuget.org as v0.9.8

thanks again!

janfolbrecht commented 7 months ago

Thank you.

út 27. 2. 2024 v 10:59 odesílatel Christian Helle @.***> napsal:

@janfolbrecht https://github.com/janfolbrecht this contribution is now released to nuget.org https://www.nuget.org/packages/Refitter/0.9.8 as v0.9.8 https://github.com/christianhelle/refitter/releases/tag/0.9.8

thanks again!

— Reply to this email directly, view it on GitHub https://github.com/christianhelle/refitter/pull/322#issuecomment-1966185478, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKB3O3BG3XSHIMEEGDVJMWLYVWU6XAVCNFSM6AAAAABDTUFMDGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRWGE4DKNBXHA . You are receiving this because you were mentioned.Message ID: @.***>