[fixed] Two instances of UserProfileDataManagement in BackendExtension? (1 Viewer)

[MJGraf]

While I was trying to get a better overview on the UPnP subsystem, I found this in BackendExtension.RegisterBackendServices (line 65):

logger.Debug("BackendExtension: Registering IUserProfileDataManagement service");
ServiceRegistration.Set<IUserProfileDataManagement>(new UserProfileDataManagement());
ServiceRegistration.Set<UserProfileDataManagement>(new UserProfileDataManagement());

I can't believe that this is intended. One of them seems redundant.
In StartupBackendServices, we only have:

ServiceRegistration.Get<UserProfileDataManagement>().Startup();

and in ShutdownBackendServices there is only:

ServiceRegistration.Get<UserProfileDataManagement>().Shutdown();

For all other services we always register the Interface - not the class. So if I'm right, we should probably remove the registration of the class and later startup and shutdown the Interface instead of the class.
Michael

 

[MJGraf]

Oh, and to make it worse: In BackendExtension.DisposeBackendServices we only remove and dispose of the instance that was registered by way of the Interface:

logger.Debug("BackendExtension: Removing IUserProfileDataManagement service");
ServiceRegistration.RemoveAndDispose<IUserProfileDataManagement>();

It has been there ever since Albert comitted the UserProfileDataManagement (https://github.com/MediaPortal/MediaPortal-2/commit/3221b75369415ddac0b85ac061bb392962a51ee9).

And even worse:
We use Get<UserProfileDataManagement>() only in the BackendExtensions class.
(https://github.com/MediaPortal/MediaPortal-2/search?q="Get<UserProfileDataManagement>"&type=Code)
Outside of this class, we only access Get<IUserProfileDataManagement>()
(https://github.com/MediaPortal/MediaPortal-2/search?q="Get<IUserProfileDataManagement>"&type=Code)
So if my understanding is correct, we use a UserProfileDataManagement object which has never been started up...

@morpheus_xx: Could you please have a short look at this? If you agree, I will try to commit a patch into a separate branch... Thanks!

 

[morpheus_xx]

Looks like a bug

I think you are right, we should only register, get, startup and shutdown services only on their interface

 

[MJGraf]

Just pushed a patch to GitHub.
Startup and Shutdown were not part of the Interface so I first thought I should extend the interface by these methods. However, the interface is not only implemented on the server side, it is also implemented in the respective proxy class on the client side. Obviously we don't want the client to be able to shut down the service. So I thought it would not be a really clean solution to add the methods to the interface and only implement it on the server side (and throw a not implemented exception on the client side). The route I took was to cast the Interface to the implementing class just for Startup and Shutdown. Seemed cleaner to me... Additionally I cleaned up the implementing class a bit and removed unnecessary code (in a separate commit).
For me it still works as expected. I think the only part in which we use the UserProfileDataManagement so far is for the resume playback function. Could someone please confirm with the attached MediaPortal.Backend.dll on the server side that resume playback of videos still works?
Thanks, Michael

PS: and now off to SQLite for SlimTVNative...

 

[breese]

Resume.... As in start a movie and then Pause it or Stop the movie and then start the same movie?
Expected to show a box to Resume or Start at XX:xx point correct?

 

[MJGraf]

Yep, exactly.
You're amazing, Breeze. Thanks for your great work!

 

[breese]

Here you go.....
All went as expected

 

[morpheus_xx]

Just pushed a patch to GitHub.
Startup and Shutdown were not part of the Interface so I first thought I should extend the interface by these methods. However, the interface is not only implemented on the server side, it is also implemented in the respective proxy class on the client side. Obviously we don't want the client to be able to shut down the service. So I thought it would not be a really clean solution to add the methods to the interface and only implement it on the server side (and throw a not implemented exception on the client side). The route I took was to cast the Interface to the implementing class just for Startup and Shutdown. Seemed cleaner to me... Additionally I cleaned up the implementing class a bit and removed unnecessary code (in a separate commit).
For me it still works as expected. I think the only part in which we use the UserProfileDataManagement so far is for the resume playback function. Could someone please confirm with the attached MediaPortal.Backend.dll on the server side that resume playback of videos still works?
Thanks, Michael

PS: and now off to SQLite for SlimTVNative...

Thanks, but please use a branch the next time

 

[MJGraf]

but please use a branch the ne

Oh, shhh.... Sorry - first rule for the future: Never push after a 15 hours working day...