Pax User Admin

Preferences Storage provider can't handle service dynamics

Details

  • Type: Improvement Improvement
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 0.0.1
  • Fix Version/s: None
  • Component/s: Preferences Provider
  • Labels:
    None

Description

The Preferences based storage implementation can't handle unavailability of the Preferences service at startup. In addition I assume that when another higher ranking Preferences service came along the service would simple switch persistence store in the middle of operation.

I suggest using an adapter approach where a StorageProvider will be registered for either one or each available Preferences service and let it bind to that service lifecycle. This makes things more robust as it wont fail when the Preferences service is registered at a later time or when multiple Preferences services are around.

My preference would be to adapt to each as it makes the least assumptions on the environment. I'll can work out a patch. WDYT?

Activity

Hide
Matthias Küspert added a comment -

Sounds good. I never got time to work more on the dynamic behaviour. Feel free to add what's missing

Show
Matthias Küspert added a comment - Sounds good. I never got time to work more on the dynamic behaviour. Feel free to add what's missing
Hide
Bram de Kruijff added a comment - - edited

Alright, I've pushed the changes. There are a few thing to note here:

1) Strictly speaking this changes the behavior in cases where there are multiple PreferencesService registrations. In the old situation this would lead to one StorageProvider (probably) using the higher ranked one. In the new situation there will be multiple StorageProvider registrations, each using one distinct PreferencesService. I don't think that's a big issue because in such an environment it was probably broken.

2) Although, now there will be multiple StorageProvider registrations this does not amount to much because they will all have type "Preferences". As UserAdminService keeps a map of providers based on the type it will only use the first one to appear. A possible additional change to address this is to register the storage provider with type "Preference <PreferencesService service.id>. As a result we could cover the use case where each PreferencesService registration leads to a new UserAdminService registration.

3) I have tested this using the PAX Exam2 bases integration framework we have at Amdatu. It mya be a good idea to set one up for this projects as well?

EDIT: Sorry, I totally missed the fact that there is a PAX Exam1 itest module because it is not in default reactor. Unfortunately it doesn't build for me.

Let me know what you think!

Show
Bram de Kruijff added a comment - - edited Alright, I've pushed the changes. There are a few thing to note here: 1) Strictly speaking this changes the behavior in cases where there are multiple PreferencesService registrations. In the old situation this would lead to one StorageProvider (probably) using the higher ranked one. In the new situation there will be multiple StorageProvider registrations, each using one distinct PreferencesService. I don't think that's a big issue because in such an environment it was probably broken. 2) Although, now there will be multiple StorageProvider registrations this does not amount to much because they will all have type "Preferences". As UserAdminService keeps a map of providers based on the type it will only use the first one to appear. A possible additional change to address this is to register the storage provider with type "Preference <PreferencesService service.id>. As a result we could cover the use case where each PreferencesService registration leads to a new UserAdminService registration. 3) I have tested this using the PAX Exam2 bases integration framework we have at Amdatu. It mya be a good idea to set one up for this projects as well? EDIT: Sorry, I totally missed the fact that there is a PAX Exam1 itest module because it is not in default reactor. Unfortunately it doesn't build for me. Let me know what you think!

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated: