Stability improvement: #92
Conversation
Processing in separate threat may cause racing condition and timeouts in browser. By putting processing in original threat we get a more sequential processing and more stability.
|
Isn't it blocking the UI Thread then? Will have a look at it at the weekend |
|
If the server is started on UI thread: yes! Small note: in my application i have a kind of "authentication factory" which delivers the authentication token in a awaitable call. The method takes care about starting the server, sending the request, handle the response and transform it into a result which is then returned to the user once it has completed. |
+ New: AuthenticationFactory for awaitatble getting of fully initalized SpotifyWebApi - Changed demo-app to usage of AuthenticationFactory
|
As mentioned above: |
|
Nice, I like the idea @jholzer Some thoughts: What do you think about that? I could throw in a possible implementation next week 👍 |
Proper Re-Use is now possible
|
Hmmm... The problem is, that "AuthenticationFactory" in fact return a finished "SpotifyWebAPI" object while "ClientCredentials" just return a "Token", so does "AutorizationCodeAuth". Maybe the name "AuthenticationFactory" is a little missleading. |
So IDisposable is introduced which handles the above...
…: no more chance of deadlocks in StreamReadLine) - Flush output stream and close it
| @@ -1,112 +1,5 @@ | |||
| # Build Folders (you can keep bin if you'd like, to store dlls and pdbs) | |||
There was a problem hiding this comment.
Could you restore this file? I guess VS accidently overwrote it.
|
However, I still think we could add another 2 Factories for the other ones, since these could also provide a finished But you're right, it's hard finding Common-elements since they behave very different. Maybe just start with Gonna have a deeper look next week. |
… what the class does...)
|
Yes,of course the other stuff should be wrapped in factories. |
SpotifyAPI/Local/SpotifyLocalAPI.cs
Outdated
| { | ||
| _rh = new RemoteHandler(); | ||
|
|
||
| AttachTimer(50); |
There was a problem hiding this comment.
You could make this shorter with:
public SpotifyLocalAPI(int timerIntervall = 50)
{
_rh = new RemoteHandler();
AttachTimer(timerIntervall);
}
|
I added some comments mostly relating style-choices. Regarding other Factories: |
SpotifyAPI/Web/Auth/WebApiFactory.cs
Outdated
|
|
||
| namespace SpotifyAPI.Web.Auth | ||
| { | ||
| public class WebApiFactory |
There was a problem hiding this comment.
Also, this should be WebAPIFactory (as well as the filename).
There was a problem hiding this comment.
Just commited the code style changes as requested. (Most were due to my ReSharper-settings... ;-) )
There was a problem hiding this comment.
I'm mostly following the default ReSharper settings as well, but the API lasted from the beginning. 😜
Gonna merge this tomorrow 👍
|
Also added a note at the docs |
2 participants
Processing in separate threat may cause racing condition and timeouts in browser.
By putting processing in original threat we get a more sequential processing and more stability.