You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Ralph Goers <Ra...@dslextreme.com> on 2005/01/18 19:32:35 UTC
Re: svn commit: r125515 -
Carsten -
I'm trying to understand what is up here.
There used to be a method called storeProfile() in AbstractProfileManager.
My recollection is that in AuthenticationProfileManager storeProfile()
called saveUserLayout() and saveUserCopletInstance(). Each of those
methods only stored the object in question. Now storeProfile is gone and
saveUserLayout saves both the layout and the copletInstanceData. While
more efficient, it doesn't seem right, unless saveUserLayout is renamed
saveUserProfile.
Ralph
cziegeler@apache.org said:
> Author: cziegeler
> Date: Tue Jan 18 08:33:45 2005
> New Revision: 125515
>
> URL: http://svn.apache.org/viewcvs?view=rev&rev=125515
> Log:
> Readd missing code
> Modified:
> cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java
>
> Modified:
> cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java
> Url:
> http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java?view=diff&rev=125515&p1=cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java&r1=125514&p2=cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java&r2=125515
> ==============================================================================
> ---
> cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java (original)
> +++
> cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java Tue
> Jan 18 08:33:45 2005
> @@ -185,39 +185,44 @@
> }
> }
>
> + /* (non-Javadoc)
> + * @see
> org.apache.cocoon.portal.profile.ProfileManager#saveUserLayout(java.lang.String)
> + */
> public void saveUserLayout(String layoutKey) {
> ProfileLS adapter = null;
> PortalService service = null;
> try {
> adapter = (ProfileLS) this.manager.lookup(ProfileLS.ROLE);
> service = (PortalService)
> this.manager.lookup(PortalService.ROLE);
> - if (layoutKey == null) {
> + if ( layoutKey == null ) {
> layoutKey = service.getDefaultLayoutKey();
> }
> -
> +
> RequestState state = this.getRequestState();
> UserHandler handler = state.getHandler();
>
> HashMap parameters = new HashMap();
> parameters.put("type", "user");
> - parameters.put("config",
> -
> state.getApplicationConfiguration().getConfiguration("portal").getChild("profiles"));
> + parameters.put("config",
> state.getApplicationConfiguration().getConfiguration("portal").getChild("profiles"));
> parameters.put("handler", handler);
> + parameters.put("profiletype", "copletinstancedata");
>
> Map key = this.buildKey(service, parameters, layoutKey,
> false);
> +
> + // save coplet instance data
> + CopletInstanceDataManager profileManager =
> ((CopletInstanceDataManager)service.getAttribute("CopletInstanceData:" +
> layoutKey));
> + adapter.saveProfile(key, parameters, profileManager);
>
> // save layout data
> parameters.put("profiletype", "layout");
> key = this.buildKey(service, parameters, layoutKey, false);
> - Layout layout = (Layout) service.getAttribute("Layout:" +
> layoutKey);
> + Layout layout = (Layout)service.getAttribute("Layout:" +
> layoutKey);
> adapter.saveProfile(key, parameters, layout);
> -
> - }
> - catch (Exception e) {
> +
> + } catch (Exception e) {
> // TODO
> throw new CascadingRuntimeException("Exception during save
> profile", e);
> - }
> - finally {
> + } finally {
> this.manager.release(adapter);
> this.manager.release(service);
> }
>
Re: svn commit: r125515 -
Posted by Ralph Goers <Ra...@dslextreme.com>.
AbstractProfileManager still has saveUserProfiles(). It is implemented as
I remembered, by calling saveUserLayout() and saveUserCopletInstances().
So the effect of the way the code is now is that the copletinstancedata
will be saved twice on a call to saveUserProfiles(). saveUserProfiles is
called by the SaveAction class.
I think that what you are suggesting by your comment below is simply
removing the code checked in with r125515 that saved the coplet instance
data in saveUserLayout? If so, I'm OK with that.
Ralph
Carsten Ziegeler said:
>
> Ah, ok - this way makes sense of course. Ok, so I think we should go
> with the two methods: saveUserLayout and saveUserCopletInstances and
> implement them accordingly.
>
> If you're ok with it, I can fix it tomorrow.
>
> Carsten
Re: svn commit: r125515 -
Posted by Carsten Ziegeler <cz...@apache.org>.
Ralph Goers wrote:
> Darn - email problems. I accidently deleted the message I wanted to reply
> to.
>
> Anyway, saveUserCopletInstance is the method I really care about. I don't
> think anything was ever actually calling the other save methods. When I
> added support for storing JSR-168 Portlet preferences I added code to
> cause just the coplet instance data to be saved, as that is where the
> portlet preferences reside. I don't want to save the layout as well
> since, in a lot of cases, the user may not even be able to change his
> layout and it will end up creating a slew of duplicated files.
Ah, ok - this way makes sense of course. Ok, so I think we should go
with the two methods: saveUserLayout and saveUserCopletInstances and
implement them accordingly.
If you're ok with it, I can fix it tomorrow.
Carsten
>
> As for the other two methods, I tried to leave saveUserProfiles as I found
> it in terms of functionality - it did the same thing saveUserLayout is
> doing now, but accomplished it by calling saveUserLayout and
> saveUserCopletInstance.
>
> Ralph
>
Re: svn commit: r125515 -
Posted by Ralph Goers <Ra...@dslextreme.com>.
Darn - email problems. I accidently deleted the message I wanted to reply
to.
Anyway, saveUserCopletInstance is the method I really care about. I don't
think anything was ever actually calling the other save methods. When I
added support for storing JSR-168 Portlet preferences I added code to
cause just the coplet instance data to be saved, as that is where the
portlet preferences reside. I don't want to save the layout as well
since, in a lot of cases, the user may not even be able to change his
layout and it will end up creating a slew of duplicated files.
As for the other two methods, I tried to leave saveUserProfiles as I found
it in terms of functionality - it did the same thing saveUserLayout is
doing now, but accomplished it by calling saveUserLayout and
saveUserCopletInstance.
Ralph
Message text I'm replying to ----
Ah, I just noticed that the ProfileManager interface has changed as well
- there are now three save methods:
/**
* Save the profile
*/
void saveUserProfiles(String layoutKey);
/**
* Save the layout
* @param layoutKey
*/
void saveUserLayout(String layoutKey);
/**
* Save the coplet instance
* @param layoutKey
*/
void saveUserCopletInstance(String layoutKey);
Now, the intention of saveUserProfiles() is exactly to save the layout
and the instance datas as they belong together - only saving one part is
dangerous as the layout has references to the instances. So I think we
should remove the two other save methods again - we can have two
different methods in the *implementation* if you want.
WDYT?
Re: svn commit: r125515 -
Posted by Carsten Ziegeler <cz...@apache.org>.
Ah, I just noticed that the ProfileManager interface has changed as well
- there are now three save methods:
/**
* Save the profile
*/
void saveUserProfiles(String layoutKey);
/**
* Save the layout
* @param layoutKey
*/
void saveUserLayout(String layoutKey);
/**
* Save the coplet instance
* @param layoutKey
*/
void saveUserCopletInstance(String layoutKey);
Now, the intention of saveUserProfiles() is exactly to save the layout
and the instance datas as they belong together - only saving one part is
dangerous as the layout has references to the instances. So I think we
should remove the two other save methods again - we can have two
different methods in the *implementation* if you want.
WDYT?
Carsten
Carsten Ziegeler wrote:
> Hi Ralph,
>
> I'm trying to understand it as well :) I found out today, that saving
> the profile resulted in an NPE in the saveUserLayout() method. This was
> due to some missing initialization that were removed. What I didn't
> realize while readding the missing code was the name of the method -
> saveUserLayout, I assumed it was still the old one - saveUserProfile.
>
> I can't remember that storeProfile() was implemented in this way - the
> intention of storeProfile was not to persist the profile but to make an
> in-memory copy of it; unfortunately the implementation was too difficult
> so I removed the method from the interface.
>
> So, in fact this is a little bit strange as the code couldn't run this
> way (NPE) and I think the method should be renamed to saveUserProfile.
> WDYT?
>
> Carsten
>
> Ralph Goers wrote:
>
>> Carsten -
>> I'm trying to understand what is up here.
>>
>> There used to be a method called storeProfile() in
>> AbstractProfileManager.
>> My recollection is that in AuthenticationProfileManager storeProfile()
>> called saveUserLayout() and saveUserCopletInstance(). Each of those
>> methods only stored the object in question. Now storeProfile is gone and
>> saveUserLayout saves both the layout and the copletInstanceData. While
>> more efficient, it doesn't seem right, unless saveUserLayout is renamed
>> saveUserProfile.
>>
>> Ralph
>>
>>
>> cziegeler@apache.org said:
>>
>>> Author: cziegeler
>>> Date: Tue Jan 18 08:33:45 2005
>>> New Revision: 125515
>>>
>>> URL: http://svn.apache.org/viewcvs?view=rev&rev=125515
>>> Log:
>>> Readd missing code
>>> Modified:
>>>
>>> cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java
>>>
>>>
>>> Modified:
>>> cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java
>>>
>>> Url:
>>> http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java?view=diff&rev=125515&p1=cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java&r1=125514&p2=cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java&r2=125515
>>>
>>> ==============================================================================
>>>
>>> ---
>>> cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java
>>> (original)
>>> +++
>>> cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java
>>> Tue
>>> Jan 18 08:33:45 2005
>>> @@ -185,39 +185,44 @@
>>> }
>>> }
>>>
>>> + /* (non-Javadoc)
>>> + * @see
>>> org.apache.cocoon.portal.profile.ProfileManager#saveUserLayout(java.lang.String)
>>>
>>> + */
>>> public void saveUserLayout(String layoutKey) {
>>> ProfileLS adapter = null;
>>> PortalService service = null;
>>> try {
>>> adapter = (ProfileLS) this.manager.lookup(ProfileLS.ROLE);
>>> service = (PortalService)
>>> this.manager.lookup(PortalService.ROLE);
>>> - if (layoutKey == null) {
>>> + if ( layoutKey == null ) {
>>> layoutKey = service.getDefaultLayoutKey();
>>> }
>>> -
>>> +
>>> RequestState state = this.getRequestState();
>>> UserHandler handler = state.getHandler();
>>>
>>> HashMap parameters = new HashMap();
>>> parameters.put("type", "user");
>>> - parameters.put("config",
>>> -
>>> state.getApplicationConfiguration().getConfiguration("portal").getChild("profiles"));
>>>
>>> + parameters.put("config",
>>> state.getApplicationConfiguration().getConfiguration("portal").getChild("profiles"));
>>>
>>> parameters.put("handler", handler);
>>> + parameters.put("profiletype", "copletinstancedata");
>>>
>>> Map key = this.buildKey(service, parameters, layoutKey,
>>> false);
>>> +
>>> + // save coplet instance data
>>> + CopletInstanceDataManager profileManager =
>>> ((CopletInstanceDataManager)service.getAttribute("CopletInstanceData:" +
>>> layoutKey));
>>> + adapter.saveProfile(key, parameters, profileManager);
>>>
>>> // save layout data
>>> parameters.put("profiletype", "layout");
>>> key = this.buildKey(service, parameters, layoutKey, false);
>>> - Layout layout = (Layout) service.getAttribute("Layout:" +
>>> layoutKey);
>>> + Layout layout = (Layout)service.getAttribute("Layout:" +
>>> layoutKey);
>>> adapter.saveProfile(key, parameters, layout);
>>> -
>>> - }
>>> - catch (Exception e) {
>>> +
>>> + } catch (Exception e) {
>>> // TODO
>>> throw new CascadingRuntimeException("Exception during save
>>> profile", e);
>>> - }
>>> - finally {
>>> + } finally {
>>> this.manager.release(adapter);
>>> this.manager.release(service);
>>> }
>>>
>>
>>
>>
>>
>
>
>
Re: svn commit: r125515 -
Posted by Carsten Ziegeler <cz...@apache.org>.
Hi Ralph,
I'm trying to understand it as well :) I found out today, that saving
the profile resulted in an NPE in the saveUserLayout() method. This was
due to some missing initialization that were removed. What I didn't
realize while readding the missing code was the name of the method -
saveUserLayout, I assumed it was still the old one - saveUserProfile.
I can't remember that storeProfile() was implemented in this way - the
intention of storeProfile was not to persist the profile but to make an
in-memory copy of it; unfortunately the implementation was too difficult
so I removed the method from the interface.
So, in fact this is a little bit strange as the code couldn't run this
way (NPE) and I think the method should be renamed to saveUserProfile. WDYT?
Carsten
Ralph Goers wrote:
> Carsten -
> I'm trying to understand what is up here.
>
> There used to be a method called storeProfile() in AbstractProfileManager.
> My recollection is that in AuthenticationProfileManager storeProfile()
> called saveUserLayout() and saveUserCopletInstance(). Each of those
> methods only stored the object in question. Now storeProfile is gone and
> saveUserLayout saves both the layout and the copletInstanceData. While
> more efficient, it doesn't seem right, unless saveUserLayout is renamed
> saveUserProfile.
>
> Ralph
>
>
> cziegeler@apache.org said:
>
>>Author: cziegeler
>>Date: Tue Jan 18 08:33:45 2005
>>New Revision: 125515
>>
>>URL: http://svn.apache.org/viewcvs?view=rev&rev=125515
>>Log:
>>Readd missing code
>>Modified:
>> cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java
>>
>>Modified:
>>cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java
>>Url:
>>http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java?view=diff&rev=125515&p1=cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java&r1=125514&p2=cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java&r2=125515
>>==============================================================================
>>---
>>cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java (original)
>>+++
>>cocoon/branches/BRANCH_2_1_X/src/blocks/portal/java/org/apache/cocoon/portal/profile/impl/AuthenticationProfileManager.java Tue
>>Jan 18 08:33:45 2005
>>@@ -185,39 +185,44 @@
>> }
>> }
>>
>>+ /* (non-Javadoc)
>>+ * @see
>>org.apache.cocoon.portal.profile.ProfileManager#saveUserLayout(java.lang.String)
>>+ */
>> public void saveUserLayout(String layoutKey) {
>> ProfileLS adapter = null;
>> PortalService service = null;
>> try {
>> adapter = (ProfileLS) this.manager.lookup(ProfileLS.ROLE);
>> service = (PortalService)
>>this.manager.lookup(PortalService.ROLE);
>>- if (layoutKey == null) {
>>+ if ( layoutKey == null ) {
>> layoutKey = service.getDefaultLayoutKey();
>> }
>>-
>>+
>> RequestState state = this.getRequestState();
>> UserHandler handler = state.getHandler();
>>
>> HashMap parameters = new HashMap();
>> parameters.put("type", "user");
>>- parameters.put("config",
>>-
>>state.getApplicationConfiguration().getConfiguration("portal").getChild("profiles"));
>>+ parameters.put("config",
>>state.getApplicationConfiguration().getConfiguration("portal").getChild("profiles"));
>> parameters.put("handler", handler);
>>+ parameters.put("profiletype", "copletinstancedata");
>>
>> Map key = this.buildKey(service, parameters, layoutKey,
>>false);
>>+
>>+ // save coplet instance data
>>+ CopletInstanceDataManager profileManager =
>>((CopletInstanceDataManager)service.getAttribute("CopletInstanceData:" +
>>layoutKey));
>>+ adapter.saveProfile(key, parameters, profileManager);
>>
>> // save layout data
>> parameters.put("profiletype", "layout");
>> key = this.buildKey(service, parameters, layoutKey, false);
>>- Layout layout = (Layout) service.getAttribute("Layout:" +
>>layoutKey);
>>+ Layout layout = (Layout)service.getAttribute("Layout:" +
>>layoutKey);
>> adapter.saveProfile(key, parameters, layout);
>>-
>>- }
>>- catch (Exception e) {
>>+
>>+ } catch (Exception e) {
>> // TODO
>> throw new CascadingRuntimeException("Exception during save
>>profile", e);
>>- }
>>- finally {
>>+ } finally {
>> this.manager.release(adapter);
>> this.manager.release(service);
>> }
>>
>
>
>
>