Generating IObservable type response #322
Conversation
christianhelle
added
enhancement
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ 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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| settings.ReturnIObservable = true; | ||
| var generateCode = await GenerateCode(version, filename, settings); | ||
| //cannot build without it because System.Reactive package has to be installed first | ||
| generateCode += @" |
There was a problem hiding this comment.
You can add the System.Reactive package reference to the ProjectFileContents.cs file
<PackageReference Include=""System.Reactive"" Version=""6.0.0"" />so you don't need to insert a fake Unit type
|
@all-contributors please add @janfolbrecht for ideas and code |
|
I've put up a pull request to add @janfolbrecht! 🎉 |
| } | ||
|
|
||
| private string GetAsyncOperationType(bool withVoidReturnType) => | ||
| settings.ReturnIObservable | ||
| ? "IObservable" + (withVoidReturnType ? "<Unit>" : string.Empty) |
There was a problem hiding this comment.
I don't have much experience working with IObservable<T> but why do we need Unit? What does it do and what is it for?
The examples in the Refit README 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<HttpResponseMessage> GetUser(string user);
|
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
<#322 (comment)>
:
> + {
+ "Refit",
+ "System.Collections.Generic",
+ "System.Text.Json.Serialization",
+ };
+ public static string[] GetImportedNamespaces(RefitGeneratorSettings settings)
+ {
+ var namespaces = new List<string>(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
<#322 (comment)>
:
> }
+ private string GetAsyncOperationType(bool withVoidReturnType) =>
+ settings.ReturnIObservable
+ ? "IObservable" + (withVoidReturnType ? "<Unit>" : string.Empty)
I don't have much experience working with IObservable<T> 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<HttpResponseMessage> GetUser(string user);
—
Reply to this email directly, view it on GitHub
<#322 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKB3O3AGWAJGG2TR4VONFS3YUZO3ZAVCNFSM6AAAAABDTUFMDGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQOJUGQYTAOJSGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@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 |
|
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
<#322 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKB3O3HED4XQLH2W4RVQC6DYU5CEJAVCNFSM6AAAAABDTUFMDGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNJZGQ3DAMZRGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There was a problem hiding this comment.
@janfolbrecht I'll mark this pull request as approved. If you're still up for making the last changes then I can wait with merging, and I will re-review your incoming changes.
I don't mind doing the suggested changes myself if you would rather have it that way
|
Thank you. If you can make suggested changes please do it. Jan |
|
|
@janfolbrecht this contribution is now released to nuget.org as v0.9.8 thanks again! |
|
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
<#322 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKB3O3BG3XSHIMEEGDVJMWLYVWU6XAVCNFSM6AAAAABDTUFMDGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRWGE4DKNBXHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
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.