You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rave.apache.org by Chris Geer <ch...@cxtsoftware.com> on 2012/10/13 02:29:53 UTC
Model-Split Branch
I think we are at a point where the model-split work is far enough along
that we should consider merging it soon. I need to finish up refactoring
WidgetRating but after that there will be enough critical mass to merge it
into the baseline IMHO. Prior to that happening it would be good if we can
get several people to spend a little time and review the changes that have
been made. Any volunteers?
The major changes are:
1) All IDs in the interfaces have been changed from int to String. This is
to support a more flexible ID structure in the future with backends other
than JPA
2) The model has been split into several related chunks. The major chunks
are Users/People, Widget and other right now. Object relationships between
those groups have been broken and replaced with IDs. This is to support a
more modular backend eventually. Right now they are all still part of the
same JPA persistance unit but that is short term.
3) The widget model has been changed to make Widget the top level object
and WidgetComment, WidgetTag & WidgetRating are subordinate objects. These
changes include removing the widgetID attribute from the subordinate object
so that they are associated with the Widget they are attached to. We also
consolidated the various services and repositories into the
WidgetService/Repository since acting on the subordinate
object independent of the widget isn't ideal.
4) The link between RegionWidget and Widget has been replaced with
WidgetID. This has required that we populate the JSP attributes with the
list of widgets on a page (in the controller).
There are plenty of other changes we could make so if there are any you
think are crucial before the merge please point those out but I'm hoping we
can get the major changes into trunk and then iterate through the rest.
Thanks,
Chris
RE: Model-Split Branch
Posted by "Franklin, Matthew B." <mf...@mitre.org>.
>-----Original Message-----
>From: Chris Geer [mailto:chris@cxtsoftware.com]
>Sent: Friday, October 26, 2012 4:27 PM
>To: dev@rave.apache.org
>Subject: Re: Model-Split Branch
>
>Raminder,
>
>Hmmm, Matt took care of that aspect of the refactor so maybe he can talk
>about better than I.
>
>Chris
>
>On Fri, Oct 26, 2012 at 1:24 PM, Raminderjeet Singh <
>raminderjsingh@gmail.com> wrote:
>
>> Chris,
>>
>> I looked into the rave-core changes and they look good. I have question
>> related to rave-jpa, In JpaPerson we have entityid, id and username.
>> Entityid is a long auto generated number. How we are going to use id as
>> string here? Other model changes related to ManyToOne relationship make
>> sense as that will make JPA object simpler.
entityId is not part of the Person interface, so JpaPerson just returns the string representation of the Long for Id. Does this answer your question, or am I missing something?
>>
>> Thanks
>> Raminder
>>
>> On Oct 25, 2012, at 1:12 PM, Chris Geer wrote:
>>
>> > All, I will be merging this over the weekend if there are no objections.
>> > (Sunday should be 72 hours). At that time I will do the final reverse
>> merge
>> > into the branch and merge into trunk.
>> >
>> > Chris
>> >
>> > On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <ch...@cxtsoftware.com>
>> wrote:
>> >
>> >> I think we are at a point where the model-split work is far enough along
>> >> that we should consider merging it soon. I need to finish up refactoring
>> >> WidgetRating but after that there will be enough critical mass to merge
>> it
>> >> into the baseline IMHO. Prior to that happening it would be good if we
>> can
>> >> get several people to spend a little time and review the changes that
>> have
>> >> been made. Any volunteers?
>> >>
>> >> The major changes are:
>> >> 1) All IDs in the interfaces have been changed from int to String. This
>> is
>> >> to support a more flexible ID structure in the future with backends
>> other
>> >> than JPA
>> >> 2) The model has been split into several related chunks. The major
>> chunks
>> >> are Users/People, Widget and other right now. Object relationships
>> between
>> >> those groups have been broken and replaced with IDs. This is to support
>> a
>> >> more modular backend eventually. Right now they are all still part of
>> the
>> >> same JPA persistance unit but that is short term.
>> >> 3) The widget model has been changed to make Widget the top level
>object
>> >> and WidgetComment, WidgetTag & WidgetRating are subordinate
>objects.
>> These
>> >> changes include removing the widgetID attribute from the subordinate
>> object
>> >> so that they are associated with the Widget they are attached to. We
>> also
>> >> consolidated the various services and repositories into the
>> >> WidgetService/Repository since acting on the subordinate
>> >> object independent of the widget isn't ideal.
>> >> 4) The link between RegionWidget and Widget has been replaced with
>> >> WidgetID. This has required that we populate the JSP attributes with the
>> >> list of widgets on a page (in the controller).
>> >>
>> >> There are plenty of other changes we could make so if there are any you
>> >> think are crucial before the merge please point those out but I'm
>> hoping we
>> >> can get the major changes into trunk and then iterate through the rest.
>> >>
>> >> Thanks,
>> >> Chris
>> >>
>>
>>
Re: Model-Split Branch
Posted by Chris Geer <ch...@cxtsoftware.com>.
Raminder,
Hmmm, Matt took care of that aspect of the refactor so maybe he can talk
about better than I.
Chris
On Fri, Oct 26, 2012 at 1:24 PM, Raminderjeet Singh <
raminderjsingh@gmail.com> wrote:
> Chris,
>
> I looked into the rave-core changes and they look good. I have question
> related to rave-jpa, In JpaPerson we have entityid, id and username.
> Entityid is a long auto generated number. How we are going to use id as
> string here? Other model changes related to ManyToOne relationship make
> sense as that will make JPA object simpler.
>
> Thanks
> Raminder
>
> On Oct 25, 2012, at 1:12 PM, Chris Geer wrote:
>
> > All, I will be merging this over the weekend if there are no objections.
> > (Sunday should be 72 hours). At that time I will do the final reverse
> merge
> > into the branch and merge into trunk.
> >
> > Chris
> >
> > On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <ch...@cxtsoftware.com>
> wrote:
> >
> >> I think we are at a point where the model-split work is far enough along
> >> that we should consider merging it soon. I need to finish up refactoring
> >> WidgetRating but after that there will be enough critical mass to merge
> it
> >> into the baseline IMHO. Prior to that happening it would be good if we
> can
> >> get several people to spend a little time and review the changes that
> have
> >> been made. Any volunteers?
> >>
> >> The major changes are:
> >> 1) All IDs in the interfaces have been changed from int to String. This
> is
> >> to support a more flexible ID structure in the future with backends
> other
> >> than JPA
> >> 2) The model has been split into several related chunks. The major
> chunks
> >> are Users/People, Widget and other right now. Object relationships
> between
> >> those groups have been broken and replaced with IDs. This is to support
> a
> >> more modular backend eventually. Right now they are all still part of
> the
> >> same JPA persistance unit but that is short term.
> >> 3) The widget model has been changed to make Widget the top level object
> >> and WidgetComment, WidgetTag & WidgetRating are subordinate objects.
> These
> >> changes include removing the widgetID attribute from the subordinate
> object
> >> so that they are associated with the Widget they are attached to. We
> also
> >> consolidated the various services and repositories into the
> >> WidgetService/Repository since acting on the subordinate
> >> object independent of the widget isn't ideal.
> >> 4) The link between RegionWidget and Widget has been replaced with
> >> WidgetID. This has required that we populate the JSP attributes with the
> >> list of widgets on a page (in the controller).
> >>
> >> There are plenty of other changes we could make so if there are any you
> >> think are crucial before the merge please point those out but I'm
> hoping we
> >> can get the major changes into trunk and then iterate through the rest.
> >>
> >> Thanks,
> >> Chris
> >>
>
>
Re: Model-Split Branch
Posted by Raminderjeet Singh <ra...@gmail.com>.
Chris,
I looked into the rave-core changes and they look good. I have question related to rave-jpa, In JpaPerson we have entityid, id and username. Entityid is a long auto generated number. How we are going to use id as string here? Other model changes related to ManyToOne relationship make sense as that will make JPA object simpler.
Thanks
Raminder
On Oct 25, 2012, at 1:12 PM, Chris Geer wrote:
> All, I will be merging this over the weekend if there are no objections.
> (Sunday should be 72 hours). At that time I will do the final reverse merge
> into the branch and merge into trunk.
>
> Chris
>
> On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <ch...@cxtsoftware.com> wrote:
>
>> I think we are at a point where the model-split work is far enough along
>> that we should consider merging it soon. I need to finish up refactoring
>> WidgetRating but after that there will be enough critical mass to merge it
>> into the baseline IMHO. Prior to that happening it would be good if we can
>> get several people to spend a little time and review the changes that have
>> been made. Any volunteers?
>>
>> The major changes are:
>> 1) All IDs in the interfaces have been changed from int to String. This is
>> to support a more flexible ID structure in the future with backends other
>> than JPA
>> 2) The model has been split into several related chunks. The major chunks
>> are Users/People, Widget and other right now. Object relationships between
>> those groups have been broken and replaced with IDs. This is to support a
>> more modular backend eventually. Right now they are all still part of the
>> same JPA persistance unit but that is short term.
>> 3) The widget model has been changed to make Widget the top level object
>> and WidgetComment, WidgetTag & WidgetRating are subordinate objects. These
>> changes include removing the widgetID attribute from the subordinate object
>> so that they are associated with the Widget they are attached to. We also
>> consolidated the various services and repositories into the
>> WidgetService/Repository since acting on the subordinate
>> object independent of the widget isn't ideal.
>> 4) The link between RegionWidget and Widget has been replaced with
>> WidgetID. This has required that we populate the JSP attributes with the
>> list of widgets on a page (in the controller).
>>
>> There are plenty of other changes we could make so if there are any you
>> think are crucial before the merge please point those out but I'm hoping we
>> can get the major changes into trunk and then iterate through the rest.
>>
>> Thanks,
>> Chris
>>
Re: Model-Split Branch
Posted by Chris Geer <ch...@cxtsoftware.com>.
On Tue, Oct 30, 2012 at 10:57 AM, Chris Geer <ch...@cxtsoftware.com> wrote:
> On Tue, Oct 30, 2012 at 9:11 AM, Ate Douma <at...@douma.nu> wrote:
>
>> On 10/29/2012 05:11 AM, Chris Geer wrote:
>>
>>> Branch is now merged into trunk. The only change I had to make was to
>>> the OmdlOutputAdapter class where I had to comment out the @Component
>>> annotation. This was because that class now needed a constructor that
>>> took
>>> several services to function and @Autowire wasn't working due to order of
>>> creation issues I believe. The only place I could find it was used in the
>>> code was a place it was manually constructed so I think we are fine but I
>>> may have missed something. I ran all the unit tests, integration tests
>>> and
>>> also spot checked a lot of the capabilities.
>>>
>>
>> Hi Chris,
>>
>> While trying to reconcile these model-split changes against trunk with
>> the content-services sandbox, I encountered a few issues, all related to
>> the RegionWidget - Widget separation.
>> One minor issue is I think a simple merge error but others aspects are
>> more structural.
>>
>> First of all, currently Rave breaks when accessed from a mobile device:
>> mobile rendering now throws a JSP rendering exception because the
>> <rave:render-widget/> tag has an extra and required widget parameter which
>> hasn't been set (mobile.jsp #78).
>>
>
> I will try and fix.
>
>>
>> Fixed RAVE-839
> Another issue is that the Profile page no longer renders (static) widgets
>> because the ProfileController doesn't derive and saves the list of widgets
>> in the page to the model (as PageController does). I think you did have
>> this in the branch but it got lost during the merge back into trunk.
>>
>
> I will try and fix.
>
>>
>> Fixed RAVE-840
> This functionality, retrieving and deriving the widgets from the page
>> region(s) RegionWidgets and storing them as a list on the model so they are
>> available during rendering, I'm not very happy with. Especially not when it
>> is stored as a list. A map (WidgetId->Widget) might be better already so we
>> don't need to iterate on the whole list to find back a matching single
>> widget, as for example now done in region.tag.
>> But maybe an even cleaner alternative would be retrieving a Widget 'on
>> demand' through a custom tag itself, similar to the PersonTag does?
>> And tags as RegionWidgetTag then can do this internally and then wouldn't
>> need the extra widget parameter.
>>
>> Thoughts?
>
>
>>
>>> After integration I did find one bug
>>> RAVE-837<https://issues.**apache.org/jira/browse/RAVE-**837<https://issues.apache.org/jira/browse/RAVE-837>>
>>> that
>>>
>>> I will try and fix in the next day or so. Please file tickets for any
>>> other
>>> bugs and we'll squash them.
>>>
>>> Just remember now, all the IDs in the interfaces are Strings now, not
>>> Longs. Longs are still used in the JPA implementation and converted to
>>> strings at the interface level.
>>>
>>> @Jasha - I fixed the category problem prior to merge.
>>>
>>> Chris
>>>
>>> On Fri, Oct 26, 2012 at 4:30 AM, Jasha Joachimsthal <jasha@apache.org
>>> >wrote:
>>>
>>> Hi Chris,
>>>>
>>>> I did a few tests and found the following things:
>>>>
>>>> Widget Store shows a log entry about tags:
>>>> [INFO] [talledLocalContainer] 465793 ravePersistenceUnit WARN
>>>> [http-8080-1] openjpa.Runtime - Supplied user parameters "[widgetId,
>>>> tagId]" do not match expected parameters "[widgetId]" for the prepared
>>>> query "PreparedQuery: [select t from JpaWidgetTag t where
>>>> t.widgetId=:widgetId and t.tagId=:tagId] --> [SELECT t0.entity_id,
>>>> t0.created_date, t0.tag_id, t0.user_id, t0.widget_id FROM widget_tag t0
>>>> WHERE (t0.widget_id = ? AND t0.tag_id IS NULL)]".
>>>>
>>>> In the Widget store if you select a category and then select the empty
>>>> category you get an error page:
>>>> URL:
>>>>
>>>> http://localhost:8080/portal/**app/store/category?categoryId=**
>>>> 0&referringPageId=1(logged<http://localhost:8080/portal/app/store/category?categoryId=0&referringPageId=1(logged>
>>>> in as canonical)
>>>> [WARNING] [talledLocalContainer] java.lang.NullPointerException
>>>> [WARNING] [talledLocalContainer] at
>>>>
>>>> org.apache.rave.portal.**service.impl.**DefaultWidgetService.**
>>>> getWidgetsByCategory(**DefaultWidgetService.java:181)
>>>>
>>>> The integration tests passed except the last one (OpenID login), but
>>>> that
>>>> one has been fixed in the trunk.
>>>> No real blockers IMO.
>>>>
>>>> Jasha
>>>>
>>>>
>>>> On 25 October 2012 19:12, Chris Geer <ch...@cxtsoftware.com> wrote:
>>>>
>>>> All, I will be merging this over the weekend if there are no
>>>>> objections.
>>>>> (Sunday should be 72 hours). At that time I will do the final reverse
>>>>>
>>>> merge
>>>>
>>>>> into the branch and merge into trunk.
>>>>>
>>>>> Chris
>>>>>
>>>>> On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <ch...@cxtsoftware.com>
>>>>>
>>>> wrote:
>>>>
>>>>>
>>>>> I think we are at a point where the model-split work is far enough
>>>>>>
>>>>> along
>>>>
>>>>> that we should consider merging it soon. I need to finish up
>>>>>>
>>>>> refactoring
>>>>
>>>>> WidgetRating but after that there will be enough critical mass to merge
>>>>>>
>>>>> it
>>>>>
>>>>>> into the baseline IMHO. Prior to that happening it would be good if we
>>>>>>
>>>>> can
>>>>>
>>>>>> get several people to spend a little time and review the changes that
>>>>>>
>>>>> have
>>>>>
>>>>>> been made. Any volunteers?
>>>>>>
>>>>>> The major changes are:
>>>>>> 1) All IDs in the interfaces have been changed from int to String.
>>>>>> This
>>>>>>
>>>>> is
>>>>>
>>>>>> to support a more flexible ID structure in the future with backends
>>>>>>
>>>>> other
>>>>
>>>>> than JPA
>>>>>> 2) The model has been split into several related chunks. The major
>>>>>>
>>>>> chunks
>>>>
>>>>> are Users/People, Widget and other right now. Object relationships
>>>>>>
>>>>> between
>>>>>
>>>>>> those groups have been broken and replaced with IDs. This is to
>>>>>>
>>>>> support a
>>>>
>>>>> more modular backend eventually. Right now they are all still part of
>>>>>>
>>>>> the
>>>>
>>>>> same JPA persistance unit but that is short term.
>>>>>> 3) The widget model has been changed to make Widget the top level
>>>>>>
>>>>> object
>>>>
>>>>> and WidgetComment, WidgetTag & WidgetRating are subordinate objects.
>>>>>>
>>>>> These
>>>>>
>>>>>> changes include removing the widgetID attribute from the subordinate
>>>>>>
>>>>> object
>>>>>
>>>>>> so that they are associated with the Widget they are attached to. We
>>>>>>
>>>>> also
>>>>
>>>>> consolidated the various services and repositories into the
>>>>>> WidgetService/Repository since acting on the subordinate
>>>>>> object independent of the widget isn't ideal.
>>>>>> 4) The link between RegionWidget and Widget has been replaced with
>>>>>> WidgetID. This has required that we populate the JSP attributes with
>>>>>>
>>>>> the
>>>>
>>>>> list of widgets on a page (in the controller).
>>>>>>
>>>>>> There are plenty of other changes we could make so if there are any
>>>>>> you
>>>>>> think are crucial before the merge please point those out but I'm
>>>>>>
>>>>> hoping
>>>>
>>>>> we
>>>>>
>>>>>> can get the major changes into trunk and then iterate through the
>>>>>> rest.
>>>>>>
>>>>>> Thanks,
>>>>>> Chris
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
Re: Model-Split Branch
Posted by Chris Geer <ch...@cxtsoftware.com>.
On Tue, Oct 30, 2012 at 9:11 AM, Ate Douma <at...@douma.nu> wrote:
> On 10/29/2012 05:11 AM, Chris Geer wrote:
>
>> Branch is now merged into trunk. The only change I had to make was to
>> the OmdlOutputAdapter class where I had to comment out the @Component
>> annotation. This was because that class now needed a constructor that took
>> several services to function and @Autowire wasn't working due to order of
>> creation issues I believe. The only place I could find it was used in the
>> code was a place it was manually constructed so I think we are fine but I
>> may have missed something. I ran all the unit tests, integration tests and
>> also spot checked a lot of the capabilities.
>>
>
> Hi Chris,
>
> While trying to reconcile these model-split changes against trunk with the
> content-services sandbox, I encountered a few issues, all related to the
> RegionWidget - Widget separation.
> One minor issue is I think a simple merge error but others aspects are
> more structural.
>
> First of all, currently Rave breaks when accessed from a mobile device:
> mobile rendering now throws a JSP rendering exception because the
> <rave:render-widget/> tag has an extra and required widget parameter which
> hasn't been set (mobile.jsp #78).
>
I will try and fix.
>
> Another issue is that the Profile page no longer renders (static) widgets
> because the ProfileController doesn't derive and saves the list of widgets
> in the page to the model (as PageController does). I think you did have
> this in the branch but it got lost during the merge back into trunk.
>
I will try and fix.
>
> This functionality, retrieving and deriving the widgets from the page
> region(s) RegionWidgets and storing them as a list on the model so they are
> available during rendering, I'm not very happy with. Especially not when it
> is stored as a list. A map (WidgetId->Widget) might be better already so we
> don't need to iterate on the whole list to find back a matching single
> widget, as for example now done in region.tag.
> But maybe an even cleaner alternative would be retrieving a Widget 'on
> demand' through a custom tag itself, similar to the PersonTag does?
> And tags as RegionWidgetTag then can do this internally and then wouldn't
> need the extra widget parameter.
>
> Thoughts?
>
>> After integration I did find one bug
>> RAVE-837<https://issues.**apache.org/jira/browse/RAVE-**837<https://issues.apache.org/jira/browse/RAVE-837>>
>> that
>>
>> I will try and fix in the next day or so. Please file tickets for any
>> other
>> bugs and we'll squash them.
>>
>> Just remember now, all the IDs in the interfaces are Strings now, not
>> Longs. Longs are still used in the JPA implementation and converted to
>> strings at the interface level.
>>
>> @Jasha - I fixed the category problem prior to merge.
>>
>> Chris
>>
>> On Fri, Oct 26, 2012 at 4:30 AM, Jasha Joachimsthal <jasha@apache.org
>> >wrote:
>>
>> Hi Chris,
>>>
>>> I did a few tests and found the following things:
>>>
>>> Widget Store shows a log entry about tags:
>>> [INFO] [talledLocalContainer] 465793 ravePersistenceUnit WARN
>>> [http-8080-1] openjpa.Runtime - Supplied user parameters "[widgetId,
>>> tagId]" do not match expected parameters "[widgetId]" for the prepared
>>> query "PreparedQuery: [select t from JpaWidgetTag t where
>>> t.widgetId=:widgetId and t.tagId=:tagId] --> [SELECT t0.entity_id,
>>> t0.created_date, t0.tag_id, t0.user_id, t0.widget_id FROM widget_tag t0
>>> WHERE (t0.widget_id = ? AND t0.tag_id IS NULL)]".
>>>
>>> In the Widget store if you select a category and then select the empty
>>> category you get an error page:
>>> URL:
>>>
>>> http://localhost:8080/portal/**app/store/category?categoryId=**
>>> 0&referringPageId=1(logged<http://localhost:8080/portal/app/store/category?categoryId=0&referringPageId=1(logged>
>>> in as canonical)
>>> [WARNING] [talledLocalContainer] java.lang.NullPointerException
>>> [WARNING] [talledLocalContainer] at
>>>
>>> org.apache.rave.portal.**service.impl.**DefaultWidgetService.**
>>> getWidgetsByCategory(**DefaultWidgetService.java:181)
>>>
>>> The integration tests passed except the last one (OpenID login), but that
>>> one has been fixed in the trunk.
>>> No real blockers IMO.
>>>
>>> Jasha
>>>
>>>
>>> On 25 October 2012 19:12, Chris Geer <ch...@cxtsoftware.com> wrote:
>>>
>>> All, I will be merging this over the weekend if there are no objections.
>>>> (Sunday should be 72 hours). At that time I will do the final reverse
>>>>
>>> merge
>>>
>>>> into the branch and merge into trunk.
>>>>
>>>> Chris
>>>>
>>>> On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <ch...@cxtsoftware.com>
>>>>
>>> wrote:
>>>
>>>>
>>>> I think we are at a point where the model-split work is far enough
>>>>>
>>>> along
>>>
>>>> that we should consider merging it soon. I need to finish up
>>>>>
>>>> refactoring
>>>
>>>> WidgetRating but after that there will be enough critical mass to merge
>>>>>
>>>> it
>>>>
>>>>> into the baseline IMHO. Prior to that happening it would be good if we
>>>>>
>>>> can
>>>>
>>>>> get several people to spend a little time and review the changes that
>>>>>
>>>> have
>>>>
>>>>> been made. Any volunteers?
>>>>>
>>>>> The major changes are:
>>>>> 1) All IDs in the interfaces have been changed from int to String. This
>>>>>
>>>> is
>>>>
>>>>> to support a more flexible ID structure in the future with backends
>>>>>
>>>> other
>>>
>>>> than JPA
>>>>> 2) The model has been split into several related chunks. The major
>>>>>
>>>> chunks
>>>
>>>> are Users/People, Widget and other right now. Object relationships
>>>>>
>>>> between
>>>>
>>>>> those groups have been broken and replaced with IDs. This is to
>>>>>
>>>> support a
>>>
>>>> more modular backend eventually. Right now they are all still part of
>>>>>
>>>> the
>>>
>>>> same JPA persistance unit but that is short term.
>>>>> 3) The widget model has been changed to make Widget the top level
>>>>>
>>>> object
>>>
>>>> and WidgetComment, WidgetTag & WidgetRating are subordinate objects.
>>>>>
>>>> These
>>>>
>>>>> changes include removing the widgetID attribute from the subordinate
>>>>>
>>>> object
>>>>
>>>>> so that they are associated with the Widget they are attached to. We
>>>>>
>>>> also
>>>
>>>> consolidated the various services and repositories into the
>>>>> WidgetService/Repository since acting on the subordinate
>>>>> object independent of the widget isn't ideal.
>>>>> 4) The link between RegionWidget and Widget has been replaced with
>>>>> WidgetID. This has required that we populate the JSP attributes with
>>>>>
>>>> the
>>>
>>>> list of widgets on a page (in the controller).
>>>>>
>>>>> There are plenty of other changes we could make so if there are any you
>>>>> think are crucial before the merge please point those out but I'm
>>>>>
>>>> hoping
>>>
>>>> we
>>>>
>>>>> can get the major changes into trunk and then iterate through the rest.
>>>>>
>>>>> Thanks,
>>>>> Chris
>>>>>
>>>>>
>>>>
>>>
>>
>
Re: Model-Split Branch
Posted by Chris Geer <ch...@cxtsoftware.com>.
On Tue, Oct 30, 2012 at 11:21 AM, Franklin, Matthew B.
<mf...@mitre.org>wrote:
> On 10/30/12 2:00 PM, "Chris Geer" <ch...@cxtsoftware.com> wrote:
>
> >On Tue, Oct 30, 2012 at 10:55 AM, Chris Geer <ch...@cxtsoftware.com>
> >wrote:
> >
> >> On Tue, Oct 30, 2012 at 9:48 AM, Ate Douma <at...@douma.nu> wrote:
> >>
> >>> Another issue I also experienced but just now properly verified is that
> >>> rendering a page (like canonical home page) since the merge has become
> >>> excruciating slow.
> >>>
> >>> I haven't spend any time yet investigating why this is, but I just
> >>> compared rendering canonical page 1 with the current trunk against an
> >>>svn
> >>> snapshot of just before the merge. Result: ~ 40 sec. vs. 1 sec.
> >>>
> >>
> >> I'm not having this problem. The main screen does take 3-4 seconds to
> >>load
> >> which isn't great but it's not performance like you are seeing. The
> >>first
> >> time you log into the app after a DB wipe does take a little while but
> >>I've
> >> always seen that.
> >>
> >> I think I know what's going on but I need some more time to track it
> >>down.
> >> For some reason RegionWidgetTag.doStartTag() is being called several
> >> hundred times on each page load, which now calls WidgetRepository.get()
> >> each time, which is relatively slow (instead of just pulling it from the
> >> regionwidget object). Let me debug a little and see what I can do.
> >>
> >>>
> >>> To clarify, doStartTag call DefaultRenderService.render which now calls
> >WidgetRepository.get(). End result is still way too many calls to
> >doStartTag/Render
>
> Is this a merge issue? I don't seem to remember having these issues in
> the branchŠ
>
I don't know...I don't remember having issues in the branch either but the
merges were really nasty since so many changes were made to trunk so it's
possible. I'll know more tonight.
>
>
> >
> >
> >> It seems to me something must be completely wrong here and right now the
> >>> portal isn't practically usable anymore.
> >>>
> >>> This is worrisome because I think the model-split is an important step
> >>> and amounts to a lot of work by Chris and Matt. I truly hope this is
> >>>just a
> >>> simple configuration issue and nothing fundamental.
> >>> Anyone else having a similar experience?
> >>>
> >>> Note: scheduled for tomorrow is the (start of the) release Rave 0.17,
> >>> and next week is ApacheCon EU, where both Matt and myself will have a
> >>> Rave presentation and demo. So I'd like to have a reliable release at
> >>>least
> >>> by then.
> >>>
> >>> Ate
> >>>
> >>>
> >>> On 10/30/2012 05:11 PM, Ate Douma wrote:
> >>>
> >>>> On 10/29/2012 05:11 AM, Chris Geer wrote:
> >>>>
> >>>>> Branch is now merged into trunk. The only change I had to make was to
> >>>>> the OmdlOutputAdapter class where I had to comment out the @Component
> >>>>> annotation. This was because that class now needed a constructor that
> >>>>> took
> >>>>> several services to function and @Autowire wasn't working due to
> >>>>>order
> >>>>> of
> >>>>> creation issues I believe. The only place I could find it was used in
> >>>>> the
> >>>>> code was a place it was manually constructed so I think we are fine
> >>>>>but
> >>>>> I
> >>>>> may have missed something. I ran all the unit tests, integration
> >>>>>tests
> >>>>> and
> >>>>> also spot checked a lot of the capabilities.
> >>>>>
> >>>>
> >>>> Hi Chris,
> >>>>
> >>>> While trying to reconcile these model-split changes against trunk with
> >>>> the
> >>>> content-services sandbox, I encountered a few issues, all related to
> >>>>the
> >>>> RegionWidget - Widget separation.
> >>>> One minor issue is I think a simple merge error but others aspects are
> >>>> more
> >>>> structural.
> >>>>
> >>>> First of all, currently Rave breaks when accessed from a mobile
> >>>>device:
> >>>> mobile
> >>>> rendering now throws a JSP rendering exception because the
> >>>> <rave:render-widget/>
> >>>> tag has an extra and required widget parameter which hasn't been set
> >>>> (mobile.jsp
> >>>> #78).
> >>>>
> >>>> Another issue is that the Profile page no longer renders (static)
> >>>>widgets
> >>>> because the ProfileController doesn't derive and saves the list of
> >>>> widgets in
> >>>> the page to the model (as PageController does). I think you did have
> >>>> this in the
> >>>> branch but it got lost during the merge back into trunk.
> >>>>
> >>>> This functionality, retrieving and deriving the widgets from the page
> >>>> region(s)
> >>>> RegionWidgets and storing them as a list on the model so they are
> >>>> available
> >>>> during rendering, I'm not very happy with. Especially not when it is
> >>>> stored as a
> >>>> list. A map (WidgetId->Widget) might be better already so we don't
> >>>>need
> >>>> to
> >>>> iterate on the whole list to find back a matching single widget, as
> >>>>for
> >>>> example
> >>>> now done in region.tag.
> >>>> But maybe an even cleaner alternative would be retrieving a Widget 'on
> >>>> demand'
> >>>> through a custom tag itself, similar to the PersonTag does?
> >>>> And tags as RegionWidgetTag then can do this internally and then
> >>>> wouldn't need
> >>>> the extra widget parameter.
> >>>>
> >>>> Thoughts?
> >>>>
> >>>>
> >>>>> After integration I did find one bug
> >>>>>
> >>>>>RAVE-837<https://issues.**apache.org/jira/browse/RAVE-**837<
> https://is
> >>>>>sues.apache.org/jira/browse/RAVE-837>>
> >>>>> that
> >>>>> I will try and fix in the next day or so. Please file tickets for any
> >>>>> other
> >>>>> bugs and we'll squash them.
> >>>>>
> >>>>> Just remember now, all the IDs in the interfaces are Strings now, not
> >>>>> Longs. Longs are still used in the JPA implementation and converted
> >>>>>to
> >>>>> strings at the interface level.
> >>>>>
> >>>>> @Jasha - I fixed the category problem prior to merge.
> >>>>>
> >>>>> Chris
> >>>>>
> >>>>> On Fri, Oct 26, 2012 at 4:30 AM, Jasha Joachimsthal <
> jasha@apache.org
> >>>>> >wrote:
> >>>>>
> >>>>> Hi Chris,
> >>>>>>
> >>>>>> I did a few tests and found the following things:
> >>>>>>
> >>>>>> Widget Store shows a log entry about tags:
> >>>>>> [INFO] [talledLocalContainer] 465793 ravePersistenceUnit WARN
> >>>>>> [http-8080-1] openjpa.Runtime - Supplied user parameters "[widgetId,
> >>>>>> tagId]" do not match expected parameters "[widgetId]" for the
> >>>>>>prepared
> >>>>>> query "PreparedQuery: [select t from JpaWidgetTag t where
> >>>>>> t.widgetId=:widgetId and t.tagId=:tagId] --> [SELECT t0.entity_id,
> >>>>>> t0.created_date, t0.tag_id, t0.user_id, t0.widget_id FROM
> >>>>>>widget_tag t0
> >>>>>> WHERE (t0.widget_id = ? AND t0.tag_id IS NULL)]".
> >>>>>>
> >>>>>> In the Widget store if you select a category and then select the
> >>>>>>empty
> >>>>>> category you get an error page:
> >>>>>> URL:
> >>>>>>
> >>>>>> http://localhost:8080/portal/**app/store/category?categoryId=**
> >>>>>>
> >>>>>>0&referringPageId=1(logged<
> http://localhost:8080/portal/app/store/cat
> >>>>>>egory?categoryId=0&referringPageId=1(logged>
> >>>>>>
> >>>>>> in as canonical)
> >>>>>> [WARNING] [talledLocalContainer] java.lang.NullPointerException
> >>>>>> [WARNING] [talledLocalContainer] at
> >>>>>>
> >>>>>> org.apache.rave.portal.**service.impl.**DefaultWidgetService.**
> >>>>>> getWidgetsByCategory(**DefaultWidgetService.java:181)
> >>>>>>
> >>>>>>
> >>>>>> The integration tests passed except the last one (OpenID login), but
> >>>>>> that
> >>>>>> one has been fixed in the trunk.
> >>>>>> No real blockers IMO.
> >>>>>>
> >>>>>> Jasha
> >>>>>>
> >>>>>>
> >>>>>> On 25 October 2012 19:12, Chris Geer <ch...@cxtsoftware.com> wrote:
> >>>>>>
> >>>>>> All, I will be merging this over the weekend if there are no
> >>>>>>> objections.
> >>>>>>> (Sunday should be 72 hours). At that time I will do the final
> >>>>>>>reverse
> >>>>>>>
> >>>>>> merge
> >>>>>>
> >>>>>>> into the branch and merge into trunk.
> >>>>>>>
> >>>>>>> Chris
> >>>>>>>
> >>>>>>> On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <chris@cxtsoftware.com
> >
> >>>>>>>
> >>>>>> wrote:
> >>>>>>
> >>>>>>>
> >>>>>>> I think we are at a point where the model-split work is far enough
> >>>>>>>>
> >>>>>>> along
> >>>>>>
> >>>>>>> that we should consider merging it soon. I need to finish up
> >>>>>>>>
> >>>>>>> refactoring
> >>>>>>
> >>>>>>> WidgetRating but after that there will be enough critical mass to
> >>>>>>>> merge
> >>>>>>>>
> >>>>>>> it
> >>>>>>>
> >>>>>>>> into the baseline IMHO. Prior to that happening it would be good
> >>>>>>>>if
> >>>>>>>> we
> >>>>>>>>
> >>>>>>> can
> >>>>>>>
> >>>>>>>> get several people to spend a little time and review the changes
> >>>>>>>>that
> >>>>>>>>
> >>>>>>> have
> >>>>>>>
> >>>>>>>> been made. Any volunteers?
> >>>>>>>>
> >>>>>>>> The major changes are:
> >>>>>>>> 1) All IDs in the interfaces have been changed from int to String.
> >>>>>>>> This
> >>>>>>>>
> >>>>>>> is
> >>>>>>>
> >>>>>>>> to support a more flexible ID structure in the future with
> >>>>>>>>backends
> >>>>>>>>
> >>>>>>> other
> >>>>>>
> >>>>>>> than JPA
> >>>>>>>> 2) The model has been split into several related chunks. The major
> >>>>>>>>
> >>>>>>> chunks
> >>>>>>
> >>>>>>> are Users/People, Widget and other right now. Object relationships
> >>>>>>>>
> >>>>>>> between
> >>>>>>>
> >>>>>>>> those groups have been broken and replaced with IDs. This is to
> >>>>>>>>
> >>>>>>> support a
> >>>>>>
> >>>>>>> more modular backend eventually. Right now they are all still part
> >>>>>>>of
> >>>>>>>>
> >>>>>>> the
> >>>>>>
> >>>>>>> same JPA persistance unit but that is short term.
> >>>>>>>> 3) The widget model has been changed to make Widget the top level
> >>>>>>>>
> >>>>>>> object
> >>>>>>
> >>>>>>> and WidgetComment, WidgetTag & WidgetRating are subordinate
> >>>>>>>objects.
> >>>>>>>>
> >>>>>>> These
> >>>>>>>
> >>>>>>>> changes include removing the widgetID attribute from the
> >>>>>>>>subordinate
> >>>>>>>>
> >>>>>>> object
> >>>>>>>
> >>>>>>>> so that they are associated with the Widget they are attached to.
> >>>>>>>>We
> >>>>>>>>
> >>>>>>> also
> >>>>>>
> >>>>>>> consolidated the various services and repositories into the
> >>>>>>>> WidgetService/Repository since acting on the subordinate
> >>>>>>>> object independent of the widget isn't ideal.
> >>>>>>>> 4) The link between RegionWidget and Widget has been replaced with
> >>>>>>>> WidgetID. This has required that we populate the JSP attributes
> >>>>>>>>with
> >>>>>>>>
> >>>>>>> the
> >>>>>>
> >>>>>>> list of widgets on a page (in the controller).
> >>>>>>>>
> >>>>>>>> There are plenty of other changes we could make so if there are
> >>>>>>>>any
> >>>>>>>> you
> >>>>>>>> think are crucial before the merge please point those out but I'm
> >>>>>>>>
> >>>>>>> hoping
> >>>>>>
> >>>>>>> we
> >>>>>>>
> >>>>>>>> can get the major changes into trunk and then iterate through the
> >>>>>>>> rest.
> >>>>>>>>
> >>>>>>>> Thanks,
> >>>>>>>> Chris
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
>
>
Re: Model-Split Branch
Posted by "Franklin, Matthew B." <mf...@mitre.org>.
On 10/30/12 2:00 PM, "Chris Geer" <ch...@cxtsoftware.com> wrote:
>On Tue, Oct 30, 2012 at 10:55 AM, Chris Geer <ch...@cxtsoftware.com>
>wrote:
>
>> On Tue, Oct 30, 2012 at 9:48 AM, Ate Douma <at...@douma.nu> wrote:
>>
>>> Another issue I also experienced but just now properly verified is that
>>> rendering a page (like canonical home page) since the merge has become
>>> excruciating slow.
>>>
>>> I haven't spend any time yet investigating why this is, but I just
>>> compared rendering canonical page 1 with the current trunk against an
>>>svn
>>> snapshot of just before the merge. Result: ~ 40 sec. vs. 1 sec.
>>>
>>
>> I'm not having this problem. The main screen does take 3-4 seconds to
>>load
>> which isn't great but it's not performance like you are seeing. The
>>first
>> time you log into the app after a DB wipe does take a little while but
>>I've
>> always seen that.
>>
>> I think I know what's going on but I need some more time to track it
>>down.
>> For some reason RegionWidgetTag.doStartTag() is being called several
>> hundred times on each page load, which now calls WidgetRepository.get()
>> each time, which is relatively slow (instead of just pulling it from the
>> regionwidget object). Let me debug a little and see what I can do.
>>
>>>
>>> To clarify, doStartTag call DefaultRenderService.render which now calls
>WidgetRepository.get(). End result is still way too many calls to
>doStartTag/Render
Is this a merge issue? I don't seem to remember having these issues in
the branchŠ
>
>
>> It seems to me something must be completely wrong here and right now the
>>> portal isn't practically usable anymore.
>>>
>>> This is worrisome because I think the model-split is an important step
>>> and amounts to a lot of work by Chris and Matt. I truly hope this is
>>>just a
>>> simple configuration issue and nothing fundamental.
>>> Anyone else having a similar experience?
>>>
>>> Note: scheduled for tomorrow is the (start of the) release Rave 0.17,
>>> and next week is ApacheCon EU, where both Matt and myself will have a
>>> Rave presentation and demo. So I'd like to have a reliable release at
>>>least
>>> by then.
>>>
>>> Ate
>>>
>>>
>>> On 10/30/2012 05:11 PM, Ate Douma wrote:
>>>
>>>> On 10/29/2012 05:11 AM, Chris Geer wrote:
>>>>
>>>>> Branch is now merged into trunk. The only change I had to make was to
>>>>> the OmdlOutputAdapter class where I had to comment out the @Component
>>>>> annotation. This was because that class now needed a constructor that
>>>>> took
>>>>> several services to function and @Autowire wasn't working due to
>>>>>order
>>>>> of
>>>>> creation issues I believe. The only place I could find it was used in
>>>>> the
>>>>> code was a place it was manually constructed so I think we are fine
>>>>>but
>>>>> I
>>>>> may have missed something. I ran all the unit tests, integration
>>>>>tests
>>>>> and
>>>>> also spot checked a lot of the capabilities.
>>>>>
>>>>
>>>> Hi Chris,
>>>>
>>>> While trying to reconcile these model-split changes against trunk with
>>>> the
>>>> content-services sandbox, I encountered a few issues, all related to
>>>>the
>>>> RegionWidget - Widget separation.
>>>> One minor issue is I think a simple merge error but others aspects are
>>>> more
>>>> structural.
>>>>
>>>> First of all, currently Rave breaks when accessed from a mobile
>>>>device:
>>>> mobile
>>>> rendering now throws a JSP rendering exception because the
>>>> <rave:render-widget/>
>>>> tag has an extra and required widget parameter which hasn't been set
>>>> (mobile.jsp
>>>> #78).
>>>>
>>>> Another issue is that the Profile page no longer renders (static)
>>>>widgets
>>>> because the ProfileController doesn't derive and saves the list of
>>>> widgets in
>>>> the page to the model (as PageController does). I think you did have
>>>> this in the
>>>> branch but it got lost during the merge back into trunk.
>>>>
>>>> This functionality, retrieving and deriving the widgets from the page
>>>> region(s)
>>>> RegionWidgets and storing them as a list on the model so they are
>>>> available
>>>> during rendering, I'm not very happy with. Especially not when it is
>>>> stored as a
>>>> list. A map (WidgetId->Widget) might be better already so we don't
>>>>need
>>>> to
>>>> iterate on the whole list to find back a matching single widget, as
>>>>for
>>>> example
>>>> now done in region.tag.
>>>> But maybe an even cleaner alternative would be retrieving a Widget 'on
>>>> demand'
>>>> through a custom tag itself, similar to the PersonTag does?
>>>> And tags as RegionWidgetTag then can do this internally and then
>>>> wouldn't need
>>>> the extra widget parameter.
>>>>
>>>> Thoughts?
>>>>
>>>>
>>>>> After integration I did find one bug
>>>>>
>>>>>RAVE-837<https://issues.**apache.org/jira/browse/RAVE-**837<https://is
>>>>>sues.apache.org/jira/browse/RAVE-837>>
>>>>> that
>>>>> I will try and fix in the next day or so. Please file tickets for any
>>>>> other
>>>>> bugs and we'll squash them.
>>>>>
>>>>> Just remember now, all the IDs in the interfaces are Strings now, not
>>>>> Longs. Longs are still used in the JPA implementation and converted
>>>>>to
>>>>> strings at the interface level.
>>>>>
>>>>> @Jasha - I fixed the category problem prior to merge.
>>>>>
>>>>> Chris
>>>>>
>>>>> On Fri, Oct 26, 2012 at 4:30 AM, Jasha Joachimsthal <jasha@apache.org
>>>>> >wrote:
>>>>>
>>>>> Hi Chris,
>>>>>>
>>>>>> I did a few tests and found the following things:
>>>>>>
>>>>>> Widget Store shows a log entry about tags:
>>>>>> [INFO] [talledLocalContainer] 465793 ravePersistenceUnit WARN
>>>>>> [http-8080-1] openjpa.Runtime - Supplied user parameters "[widgetId,
>>>>>> tagId]" do not match expected parameters "[widgetId]" for the
>>>>>>prepared
>>>>>> query "PreparedQuery: [select t from JpaWidgetTag t where
>>>>>> t.widgetId=:widgetId and t.tagId=:tagId] --> [SELECT t0.entity_id,
>>>>>> t0.created_date, t0.tag_id, t0.user_id, t0.widget_id FROM
>>>>>>widget_tag t0
>>>>>> WHERE (t0.widget_id = ? AND t0.tag_id IS NULL)]".
>>>>>>
>>>>>> In the Widget store if you select a category and then select the
>>>>>>empty
>>>>>> category you get an error page:
>>>>>> URL:
>>>>>>
>>>>>> http://localhost:8080/portal/**app/store/category?categoryId=**
>>>>>>
>>>>>>0&referringPageId=1(logged<http://localhost:8080/portal/app/store/cat
>>>>>>egory?categoryId=0&referringPageId=1(logged>
>>>>>>
>>>>>> in as canonical)
>>>>>> [WARNING] [talledLocalContainer] java.lang.NullPointerException
>>>>>> [WARNING] [talledLocalContainer] at
>>>>>>
>>>>>> org.apache.rave.portal.**service.impl.**DefaultWidgetService.**
>>>>>> getWidgetsByCategory(**DefaultWidgetService.java:181)
>>>>>>
>>>>>>
>>>>>> The integration tests passed except the last one (OpenID login), but
>>>>>> that
>>>>>> one has been fixed in the trunk.
>>>>>> No real blockers IMO.
>>>>>>
>>>>>> Jasha
>>>>>>
>>>>>>
>>>>>> On 25 October 2012 19:12, Chris Geer <ch...@cxtsoftware.com> wrote:
>>>>>>
>>>>>> All, I will be merging this over the weekend if there are no
>>>>>>> objections.
>>>>>>> (Sunday should be 72 hours). At that time I will do the final
>>>>>>>reverse
>>>>>>>
>>>>>> merge
>>>>>>
>>>>>>> into the branch and merge into trunk.
>>>>>>>
>>>>>>> Chris
>>>>>>>
>>>>>>> On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <ch...@cxtsoftware.com>
>>>>>>>
>>>>>> wrote:
>>>>>>
>>>>>>>
>>>>>>> I think we are at a point where the model-split work is far enough
>>>>>>>>
>>>>>>> along
>>>>>>
>>>>>>> that we should consider merging it soon. I need to finish up
>>>>>>>>
>>>>>>> refactoring
>>>>>>
>>>>>>> WidgetRating but after that there will be enough critical mass to
>>>>>>>> merge
>>>>>>>>
>>>>>>> it
>>>>>>>
>>>>>>>> into the baseline IMHO. Prior to that happening it would be good
>>>>>>>>if
>>>>>>>> we
>>>>>>>>
>>>>>>> can
>>>>>>>
>>>>>>>> get several people to spend a little time and review the changes
>>>>>>>>that
>>>>>>>>
>>>>>>> have
>>>>>>>
>>>>>>>> been made. Any volunteers?
>>>>>>>>
>>>>>>>> The major changes are:
>>>>>>>> 1) All IDs in the interfaces have been changed from int to String.
>>>>>>>> This
>>>>>>>>
>>>>>>> is
>>>>>>>
>>>>>>>> to support a more flexible ID structure in the future with
>>>>>>>>backends
>>>>>>>>
>>>>>>> other
>>>>>>
>>>>>>> than JPA
>>>>>>>> 2) The model has been split into several related chunks. The major
>>>>>>>>
>>>>>>> chunks
>>>>>>
>>>>>>> are Users/People, Widget and other right now. Object relationships
>>>>>>>>
>>>>>>> between
>>>>>>>
>>>>>>>> those groups have been broken and replaced with IDs. This is to
>>>>>>>>
>>>>>>> support a
>>>>>>
>>>>>>> more modular backend eventually. Right now they are all still part
>>>>>>>of
>>>>>>>>
>>>>>>> the
>>>>>>
>>>>>>> same JPA persistance unit but that is short term.
>>>>>>>> 3) The widget model has been changed to make Widget the top level
>>>>>>>>
>>>>>>> object
>>>>>>
>>>>>>> and WidgetComment, WidgetTag & WidgetRating are subordinate
>>>>>>>objects.
>>>>>>>>
>>>>>>> These
>>>>>>>
>>>>>>>> changes include removing the widgetID attribute from the
>>>>>>>>subordinate
>>>>>>>>
>>>>>>> object
>>>>>>>
>>>>>>>> so that they are associated with the Widget they are attached to.
>>>>>>>>We
>>>>>>>>
>>>>>>> also
>>>>>>
>>>>>>> consolidated the various services and repositories into the
>>>>>>>> WidgetService/Repository since acting on the subordinate
>>>>>>>> object independent of the widget isn't ideal.
>>>>>>>> 4) The link between RegionWidget and Widget has been replaced with
>>>>>>>> WidgetID. This has required that we populate the JSP attributes
>>>>>>>>with
>>>>>>>>
>>>>>>> the
>>>>>>
>>>>>>> list of widgets on a page (in the controller).
>>>>>>>>
>>>>>>>> There are plenty of other changes we could make so if there are
>>>>>>>>any
>>>>>>>> you
>>>>>>>> think are crucial before the merge please point those out but I'm
>>>>>>>>
>>>>>>> hoping
>>>>>>
>>>>>>> we
>>>>>>>
>>>>>>>> can get the major changes into trunk and then iterate through the
>>>>>>>> rest.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Chris
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
Re: Model-Split Branch
Posted by Chris Geer <ch...@cxtsoftware.com>.
On Tue, Oct 30, 2012 at 10:55 AM, Chris Geer <ch...@cxtsoftware.com> wrote:
> On Tue, Oct 30, 2012 at 9:48 AM, Ate Douma <at...@douma.nu> wrote:
>
>> Another issue I also experienced but just now properly verified is that
>> rendering a page (like canonical home page) since the merge has become
>> excruciating slow.
>>
>> I haven't spend any time yet investigating why this is, but I just
>> compared rendering canonical page 1 with the current trunk against an svn
>> snapshot of just before the merge. Result: ~ 40 sec. vs. 1 sec.
>>
>
> I'm not having this problem. The main screen does take 3-4 seconds to load
> which isn't great but it's not performance like you are seeing. The first
> time you log into the app after a DB wipe does take a little while but I've
> always seen that.
>
> I think I know what's going on but I need some more time to track it down.
> For some reason RegionWidgetTag.doStartTag() is being called several
> hundred times on each page load, which now calls WidgetRepository.get()
> each time, which is relatively slow (instead of just pulling it from the
> regionwidget object). Let me debug a little and see what I can do.
>
>>
>> To clarify, doStartTag call DefaultRenderService.render which now calls
WidgetRepository.get(). End result is still way too many calls to
doStartTag/Render
> It seems to me something must be completely wrong here and right now the
>> portal isn't practically usable anymore.
>>
>> This is worrisome because I think the model-split is an important step
>> and amounts to a lot of work by Chris and Matt. I truly hope this is just a
>> simple configuration issue and nothing fundamental.
>> Anyone else having a similar experience?
>>
>> Note: scheduled for tomorrow is the (start of the) release Rave 0.17,
>> and next week is ApacheCon EU, where both Matt and myself will have a
>> Rave presentation and demo. So I'd like to have a reliable release at least
>> by then.
>>
>> Ate
>>
>>
>> On 10/30/2012 05:11 PM, Ate Douma wrote:
>>
>>> On 10/29/2012 05:11 AM, Chris Geer wrote:
>>>
>>>> Branch is now merged into trunk. The only change I had to make was to
>>>> the OmdlOutputAdapter class where I had to comment out the @Component
>>>> annotation. This was because that class now needed a constructor that
>>>> took
>>>> several services to function and @Autowire wasn't working due to order
>>>> of
>>>> creation issues I believe. The only place I could find it was used in
>>>> the
>>>> code was a place it was manually constructed so I think we are fine but
>>>> I
>>>> may have missed something. I ran all the unit tests, integration tests
>>>> and
>>>> also spot checked a lot of the capabilities.
>>>>
>>>
>>> Hi Chris,
>>>
>>> While trying to reconcile these model-split changes against trunk with
>>> the
>>> content-services sandbox, I encountered a few issues, all related to the
>>> RegionWidget - Widget separation.
>>> One minor issue is I think a simple merge error but others aspects are
>>> more
>>> structural.
>>>
>>> First of all, currently Rave breaks when accessed from a mobile device:
>>> mobile
>>> rendering now throws a JSP rendering exception because the
>>> <rave:render-widget/>
>>> tag has an extra and required widget parameter which hasn't been set
>>> (mobile.jsp
>>> #78).
>>>
>>> Another issue is that the Profile page no longer renders (static) widgets
>>> because the ProfileController doesn't derive and saves the list of
>>> widgets in
>>> the page to the model (as PageController does). I think you did have
>>> this in the
>>> branch but it got lost during the merge back into trunk.
>>>
>>> This functionality, retrieving and deriving the widgets from the page
>>> region(s)
>>> RegionWidgets and storing them as a list on the model so they are
>>> available
>>> during rendering, I'm not very happy with. Especially not when it is
>>> stored as a
>>> list. A map (WidgetId->Widget) might be better already so we don't need
>>> to
>>> iterate on the whole list to find back a matching single widget, as for
>>> example
>>> now done in region.tag.
>>> But maybe an even cleaner alternative would be retrieving a Widget 'on
>>> demand'
>>> through a custom tag itself, similar to the PersonTag does?
>>> And tags as RegionWidgetTag then can do this internally and then
>>> wouldn't need
>>> the extra widget parameter.
>>>
>>> Thoughts?
>>>
>>>
>>>> After integration I did find one bug
>>>> RAVE-837<https://issues.**apache.org/jira/browse/RAVE-**837<https://issues.apache.org/jira/browse/RAVE-837>>
>>>> that
>>>> I will try and fix in the next day or so. Please file tickets for any
>>>> other
>>>> bugs and we'll squash them.
>>>>
>>>> Just remember now, all the IDs in the interfaces are Strings now, not
>>>> Longs. Longs are still used in the JPA implementation and converted to
>>>> strings at the interface level.
>>>>
>>>> @Jasha - I fixed the category problem prior to merge.
>>>>
>>>> Chris
>>>>
>>>> On Fri, Oct 26, 2012 at 4:30 AM, Jasha Joachimsthal <jasha@apache.org
>>>> >wrote:
>>>>
>>>> Hi Chris,
>>>>>
>>>>> I did a few tests and found the following things:
>>>>>
>>>>> Widget Store shows a log entry about tags:
>>>>> [INFO] [talledLocalContainer] 465793 ravePersistenceUnit WARN
>>>>> [http-8080-1] openjpa.Runtime - Supplied user parameters "[widgetId,
>>>>> tagId]" do not match expected parameters "[widgetId]" for the prepared
>>>>> query "PreparedQuery: [select t from JpaWidgetTag t where
>>>>> t.widgetId=:widgetId and t.tagId=:tagId] --> [SELECT t0.entity_id,
>>>>> t0.created_date, t0.tag_id, t0.user_id, t0.widget_id FROM widget_tag t0
>>>>> WHERE (t0.widget_id = ? AND t0.tag_id IS NULL)]".
>>>>>
>>>>> In the Widget store if you select a category and then select the empty
>>>>> category you get an error page:
>>>>> URL:
>>>>>
>>>>> http://localhost:8080/portal/**app/store/category?categoryId=**
>>>>> 0&referringPageId=1(logged<http://localhost:8080/portal/app/store/category?categoryId=0&referringPageId=1(logged>
>>>>>
>>>>> in as canonical)
>>>>> [WARNING] [talledLocalContainer] java.lang.NullPointerException
>>>>> [WARNING] [talledLocalContainer] at
>>>>>
>>>>> org.apache.rave.portal.**service.impl.**DefaultWidgetService.**
>>>>> getWidgetsByCategory(**DefaultWidgetService.java:181)
>>>>>
>>>>>
>>>>> The integration tests passed except the last one (OpenID login), but
>>>>> that
>>>>> one has been fixed in the trunk.
>>>>> No real blockers IMO.
>>>>>
>>>>> Jasha
>>>>>
>>>>>
>>>>> On 25 October 2012 19:12, Chris Geer <ch...@cxtsoftware.com> wrote:
>>>>>
>>>>> All, I will be merging this over the weekend if there are no
>>>>>> objections.
>>>>>> (Sunday should be 72 hours). At that time I will do the final reverse
>>>>>>
>>>>> merge
>>>>>
>>>>>> into the branch and merge into trunk.
>>>>>>
>>>>>> Chris
>>>>>>
>>>>>> On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <ch...@cxtsoftware.com>
>>>>>>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>> I think we are at a point where the model-split work is far enough
>>>>>>>
>>>>>> along
>>>>>
>>>>>> that we should consider merging it soon. I need to finish up
>>>>>>>
>>>>>> refactoring
>>>>>
>>>>>> WidgetRating but after that there will be enough critical mass to
>>>>>>> merge
>>>>>>>
>>>>>> it
>>>>>>
>>>>>>> into the baseline IMHO. Prior to that happening it would be good if
>>>>>>> we
>>>>>>>
>>>>>> can
>>>>>>
>>>>>>> get several people to spend a little time and review the changes that
>>>>>>>
>>>>>> have
>>>>>>
>>>>>>> been made. Any volunteers?
>>>>>>>
>>>>>>> The major changes are:
>>>>>>> 1) All IDs in the interfaces have been changed from int to String.
>>>>>>> This
>>>>>>>
>>>>>> is
>>>>>>
>>>>>>> to support a more flexible ID structure in the future with backends
>>>>>>>
>>>>>> other
>>>>>
>>>>>> than JPA
>>>>>>> 2) The model has been split into several related chunks. The major
>>>>>>>
>>>>>> chunks
>>>>>
>>>>>> are Users/People, Widget and other right now. Object relationships
>>>>>>>
>>>>>> between
>>>>>>
>>>>>>> those groups have been broken and replaced with IDs. This is to
>>>>>>>
>>>>>> support a
>>>>>
>>>>>> more modular backend eventually. Right now they are all still part of
>>>>>>>
>>>>>> the
>>>>>
>>>>>> same JPA persistance unit but that is short term.
>>>>>>> 3) The widget model has been changed to make Widget the top level
>>>>>>>
>>>>>> object
>>>>>
>>>>>> and WidgetComment, WidgetTag & WidgetRating are subordinate objects.
>>>>>>>
>>>>>> These
>>>>>>
>>>>>>> changes include removing the widgetID attribute from the subordinate
>>>>>>>
>>>>>> object
>>>>>>
>>>>>>> so that they are associated with the Widget they are attached to. We
>>>>>>>
>>>>>> also
>>>>>
>>>>>> consolidated the various services and repositories into the
>>>>>>> WidgetService/Repository since acting on the subordinate
>>>>>>> object independent of the widget isn't ideal.
>>>>>>> 4) The link between RegionWidget and Widget has been replaced with
>>>>>>> WidgetID. This has required that we populate the JSP attributes with
>>>>>>>
>>>>>> the
>>>>>
>>>>>> list of widgets on a page (in the controller).
>>>>>>>
>>>>>>> There are plenty of other changes we could make so if there are any
>>>>>>> you
>>>>>>> think are crucial before the merge please point those out but I'm
>>>>>>>
>>>>>> hoping
>>>>>
>>>>>> we
>>>>>>
>>>>>>> can get the major changes into trunk and then iterate through the
>>>>>>> rest.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Chris
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
Re: Model-Split Branch
Posted by Chris Geer <ch...@cxtsoftware.com>.
On Tue, Oct 30, 2012 at 10:55 AM, Chris Geer <ch...@cxtsoftware.com> wrote:
> On Tue, Oct 30, 2012 at 9:48 AM, Ate Douma <at...@douma.nu> wrote:
>
>> Another issue I also experienced but just now properly verified is that
>> rendering a page (like canonical home page) since the merge has become
>> excruciating slow.
>>
>> I haven't spend any time yet investigating why this is, but I just
>> compared rendering canonical page 1 with the current trunk against an svn
>> snapshot of just before the merge. Result: ~ 40 sec. vs. 1 sec.
>>
>
> I'm not having this problem. The main screen does take 3-4 seconds to load
> which isn't great but it's not performance like you are seeing. The first
> time you log into the app after a DB wipe does take a little while but I've
> always seen that.
>
> I think I know what's going on but I need some more time to track it down.
> For some reason RegionWidgetTag.doStartTag() is being called several
> hundred times on each page load, which now calls WidgetRepository.get()
> each time, which is relatively slow (instead of just pulling it from the
> regionwidget object). Let me debug a little and see what I can do.
>
>>
>> I've looking into it and after I realized my profiler was just giving me
horrible data I tracked down the problem to basically the RegionWidget ->
Widget split. Now the widget is being loaded from the repository many more
times than before. I've created RAVE-838 to track this issue and discuss
solutions. I have a crude caching solution I can commit (see ticket) but
it's not great long term.
> It seems to me something must be completely wrong here and right now the
>> portal isn't practically usable anymore.
>>
>> This is worrisome because I think the model-split is an important step
>> and amounts to a lot of work by Chris and Matt. I truly hope this is just a
>> simple configuration issue and nothing fundamental.
>> Anyone else having a similar experience?
>>
>> Note: scheduled for tomorrow is the (start of the) release Rave 0.17,
>> and next week is ApacheCon EU, where both Matt and myself will have a
>> Rave presentation and demo. So I'd like to have a reliable release at least
>> by then.
>>
>> Ate
>>
>>
>> On 10/30/2012 05:11 PM, Ate Douma wrote:
>>
>>> On 10/29/2012 05:11 AM, Chris Geer wrote:
>>>
>>>> Branch is now merged into trunk. The only change I had to make was to
>>>> the OmdlOutputAdapter class where I had to comment out the @Component
>>>> annotation. This was because that class now needed a constructor that
>>>> took
>>>> several services to function and @Autowire wasn't working due to order
>>>> of
>>>> creation issues I believe. The only place I could find it was used in
>>>> the
>>>> code was a place it was manually constructed so I think we are fine but
>>>> I
>>>> may have missed something. I ran all the unit tests, integration tests
>>>> and
>>>> also spot checked a lot of the capabilities.
>>>>
>>>
>>> Hi Chris,
>>>
>>> While trying to reconcile these model-split changes against trunk with
>>> the
>>> content-services sandbox, I encountered a few issues, all related to the
>>> RegionWidget - Widget separation.
>>> One minor issue is I think a simple merge error but others aspects are
>>> more
>>> structural.
>>>
>>> First of all, currently Rave breaks when accessed from a mobile device:
>>> mobile
>>> rendering now throws a JSP rendering exception because the
>>> <rave:render-widget/>
>>> tag has an extra and required widget parameter which hasn't been set
>>> (mobile.jsp
>>> #78).
>>>
>>> Another issue is that the Profile page no longer renders (static) widgets
>>> because the ProfileController doesn't derive and saves the list of
>>> widgets in
>>> the page to the model (as PageController does). I think you did have
>>> this in the
>>> branch but it got lost during the merge back into trunk.
>>>
>>> This functionality, retrieving and deriving the widgets from the page
>>> region(s)
>>> RegionWidgets and storing them as a list on the model so they are
>>> available
>>> during rendering, I'm not very happy with. Especially not when it is
>>> stored as a
>>> list. A map (WidgetId->Widget) might be better already so we don't need
>>> to
>>> iterate on the whole list to find back a matching single widget, as for
>>> example
>>> now done in region.tag.
>>> But maybe an even cleaner alternative would be retrieving a Widget 'on
>>> demand'
>>> through a custom tag itself, similar to the PersonTag does?
>>> And tags as RegionWidgetTag then can do this internally and then
>>> wouldn't need
>>> the extra widget parameter.
>>>
>>> Thoughts?
>>>
>>>
>>>> After integration I did find one bug
>>>> RAVE-837<https://issues.**apache.org/jira/browse/RAVE-**837<https://issues.apache.org/jira/browse/RAVE-837>>
>>>> that
>>>> I will try and fix in the next day or so. Please file tickets for any
>>>> other
>>>> bugs and we'll squash them.
>>>>
>>>> Just remember now, all the IDs in the interfaces are Strings now, not
>>>> Longs. Longs are still used in the JPA implementation and converted to
>>>> strings at the interface level.
>>>>
>>>> @Jasha - I fixed the category problem prior to merge.
>>>>
>>>> Chris
>>>>
>>>> On Fri, Oct 26, 2012 at 4:30 AM, Jasha Joachimsthal <jasha@apache.org
>>>> >wrote:
>>>>
>>>> Hi Chris,
>>>>>
>>>>> I did a few tests and found the following things:
>>>>>
>>>>> Widget Store shows a log entry about tags:
>>>>> [INFO] [talledLocalContainer] 465793 ravePersistenceUnit WARN
>>>>> [http-8080-1] openjpa.Runtime - Supplied user parameters "[widgetId,
>>>>> tagId]" do not match expected parameters "[widgetId]" for the prepared
>>>>> query "PreparedQuery: [select t from JpaWidgetTag t where
>>>>> t.widgetId=:widgetId and t.tagId=:tagId] --> [SELECT t0.entity_id,
>>>>> t0.created_date, t0.tag_id, t0.user_id, t0.widget_id FROM widget_tag t0
>>>>> WHERE (t0.widget_id = ? AND t0.tag_id IS NULL)]".
>>>>>
>>>>> In the Widget store if you select a category and then select the empty
>>>>> category you get an error page:
>>>>> URL:
>>>>>
>>>>> http://localhost:8080/portal/**app/store/category?categoryId=**
>>>>> 0&referringPageId=1(logged<http://localhost:8080/portal/app/store/category?categoryId=0&referringPageId=1(logged>
>>>>>
>>>>> in as canonical)
>>>>> [WARNING] [talledLocalContainer] java.lang.NullPointerException
>>>>> [WARNING] [talledLocalContainer] at
>>>>>
>>>>> org.apache.rave.portal.**service.impl.**DefaultWidgetService.**
>>>>> getWidgetsByCategory(**DefaultWidgetService.java:181)
>>>>>
>>>>>
>>>>> The integration tests passed except the last one (OpenID login), but
>>>>> that
>>>>> one has been fixed in the trunk.
>>>>> No real blockers IMO.
>>>>>
>>>>> Jasha
>>>>>
>>>>>
>>>>> On 25 October 2012 19:12, Chris Geer <ch...@cxtsoftware.com> wrote:
>>>>>
>>>>> All, I will be merging this over the weekend if there are no
>>>>>> objections.
>>>>>> (Sunday should be 72 hours). At that time I will do the final reverse
>>>>>>
>>>>> merge
>>>>>
>>>>>> into the branch and merge into trunk.
>>>>>>
>>>>>> Chris
>>>>>>
>>>>>> On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <ch...@cxtsoftware.com>
>>>>>>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>> I think we are at a point where the model-split work is far enough
>>>>>>>
>>>>>> along
>>>>>
>>>>>> that we should consider merging it soon. I need to finish up
>>>>>>>
>>>>>> refactoring
>>>>>
>>>>>> WidgetRating but after that there will be enough critical mass to
>>>>>>> merge
>>>>>>>
>>>>>> it
>>>>>>
>>>>>>> into the baseline IMHO. Prior to that happening it would be good if
>>>>>>> we
>>>>>>>
>>>>>> can
>>>>>>
>>>>>>> get several people to spend a little time and review the changes that
>>>>>>>
>>>>>> have
>>>>>>
>>>>>>> been made. Any volunteers?
>>>>>>>
>>>>>>> The major changes are:
>>>>>>> 1) All IDs in the interfaces have been changed from int to String.
>>>>>>> This
>>>>>>>
>>>>>> is
>>>>>>
>>>>>>> to support a more flexible ID structure in the future with backends
>>>>>>>
>>>>>> other
>>>>>
>>>>>> than JPA
>>>>>>> 2) The model has been split into several related chunks. The major
>>>>>>>
>>>>>> chunks
>>>>>
>>>>>> are Users/People, Widget and other right now. Object relationships
>>>>>>>
>>>>>> between
>>>>>>
>>>>>>> those groups have been broken and replaced with IDs. This is to
>>>>>>>
>>>>>> support a
>>>>>
>>>>>> more modular backend eventually. Right now they are all still part of
>>>>>>>
>>>>>> the
>>>>>
>>>>>> same JPA persistance unit but that is short term.
>>>>>>> 3) The widget model has been changed to make Widget the top level
>>>>>>>
>>>>>> object
>>>>>
>>>>>> and WidgetComment, WidgetTag & WidgetRating are subordinate objects.
>>>>>>>
>>>>>> These
>>>>>>
>>>>>>> changes include removing the widgetID attribute from the subordinate
>>>>>>>
>>>>>> object
>>>>>>
>>>>>>> so that they are associated with the Widget they are attached to. We
>>>>>>>
>>>>>> also
>>>>>
>>>>>> consolidated the various services and repositories into the
>>>>>>> WidgetService/Repository since acting on the subordinate
>>>>>>> object independent of the widget isn't ideal.
>>>>>>> 4) The link between RegionWidget and Widget has been replaced with
>>>>>>> WidgetID. This has required that we populate the JSP attributes with
>>>>>>>
>>>>>> the
>>>>>
>>>>>> list of widgets on a page (in the controller).
>>>>>>>
>>>>>>> There are plenty of other changes we could make so if there are any
>>>>>>> you
>>>>>>> think are crucial before the merge please point those out but I'm
>>>>>>>
>>>>>> hoping
>>>>>
>>>>>> we
>>>>>>
>>>>>>> can get the major changes into trunk and then iterate through the
>>>>>>> rest.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Chris
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
Re: Model-Split Branch
Posted by Chris Geer <ch...@cxtsoftware.com>.
On Tue, Oct 30, 2012 at 9:48 AM, Ate Douma <at...@douma.nu> wrote:
> Another issue I also experienced but just now properly verified is that
> rendering a page (like canonical home page) since the merge has become
> excruciating slow.
>
> I haven't spend any time yet investigating why this is, but I just
> compared rendering canonical page 1 with the current trunk against an svn
> snapshot of just before the merge. Result: ~ 40 sec. vs. 1 sec.
>
I'm not having this problem. The main screen does take 3-4 seconds to load
which isn't great but it's not performance like you are seeing. The first
time you log into the app after a DB wipe does take a little while but I've
always seen that.
I think I know what's going on but I need some more time to track it down.
For some reason RegionWidgetTag.doStartTag() is being called several
hundred times on each page load, which now calls WidgetRepository.get()
each time, which is relatively slow (instead of just pulling it from the
regionwidget object). Let me debug a little and see what I can do.
>
> It seems to me something must be completely wrong here and right now the
> portal isn't practically usable anymore.
>
> This is worrisome because I think the model-split is an important step and
> amounts to a lot of work by Chris and Matt. I truly hope this is just a
> simple configuration issue and nothing fundamental.
> Anyone else having a similar experience?
>
> Note: scheduled for tomorrow is the (start of the) release Rave 0.17,
> and next week is ApacheCon EU, where both Matt and myself will have a Rave
> presentation and demo. So I'd like to have a reliable release at least by
> then.
>
> Ate
>
>
> On 10/30/2012 05:11 PM, Ate Douma wrote:
>
>> On 10/29/2012 05:11 AM, Chris Geer wrote:
>>
>>> Branch is now merged into trunk. The only change I had to make was to
>>> the OmdlOutputAdapter class where I had to comment out the @Component
>>> annotation. This was because that class now needed a constructor that
>>> took
>>> several services to function and @Autowire wasn't working due to order of
>>> creation issues I believe. The only place I could find it was used in the
>>> code was a place it was manually constructed so I think we are fine but I
>>> may have missed something. I ran all the unit tests, integration tests
>>> and
>>> also spot checked a lot of the capabilities.
>>>
>>
>> Hi Chris,
>>
>> While trying to reconcile these model-split changes against trunk with the
>> content-services sandbox, I encountered a few issues, all related to the
>> RegionWidget - Widget separation.
>> One minor issue is I think a simple merge error but others aspects are
>> more
>> structural.
>>
>> First of all, currently Rave breaks when accessed from a mobile device:
>> mobile
>> rendering now throws a JSP rendering exception because the
>> <rave:render-widget/>
>> tag has an extra and required widget parameter which hasn't been set
>> (mobile.jsp
>> #78).
>>
>> Another issue is that the Profile page no longer renders (static) widgets
>> because the ProfileController doesn't derive and saves the list of
>> widgets in
>> the page to the model (as PageController does). I think you did have this
>> in the
>> branch but it got lost during the merge back into trunk.
>>
>> This functionality, retrieving and deriving the widgets from the page
>> region(s)
>> RegionWidgets and storing them as a list on the model so they are
>> available
>> during rendering, I'm not very happy with. Especially not when it is
>> stored as a
>> list. A map (WidgetId->Widget) might be better already so we don't need to
>> iterate on the whole list to find back a matching single widget, as for
>> example
>> now done in region.tag.
>> But maybe an even cleaner alternative would be retrieving a Widget 'on
>> demand'
>> through a custom tag itself, similar to the PersonTag does?
>> And tags as RegionWidgetTag then can do this internally and then wouldn't
>> need
>> the extra widget parameter.
>>
>> Thoughts?
>>
>>
>>> After integration I did find one bug
>>> RAVE-837<https://issues.**apache.org/jira/browse/RAVE-**837<https://issues.apache.org/jira/browse/RAVE-837>>
>>> that
>>> I will try and fix in the next day or so. Please file tickets for any
>>> other
>>> bugs and we'll squash them.
>>>
>>> Just remember now, all the IDs in the interfaces are Strings now, not
>>> Longs. Longs are still used in the JPA implementation and converted to
>>> strings at the interface level.
>>>
>>> @Jasha - I fixed the category problem prior to merge.
>>>
>>> Chris
>>>
>>> On Fri, Oct 26, 2012 at 4:30 AM, Jasha Joachimsthal <jasha@apache.org
>>> >wrote:
>>>
>>> Hi Chris,
>>>>
>>>> I did a few tests and found the following things:
>>>>
>>>> Widget Store shows a log entry about tags:
>>>> [INFO] [talledLocalContainer] 465793 ravePersistenceUnit WARN
>>>> [http-8080-1] openjpa.Runtime - Supplied user parameters "[widgetId,
>>>> tagId]" do not match expected parameters "[widgetId]" for the prepared
>>>> query "PreparedQuery: [select t from JpaWidgetTag t where
>>>> t.widgetId=:widgetId and t.tagId=:tagId] --> [SELECT t0.entity_id,
>>>> t0.created_date, t0.tag_id, t0.user_id, t0.widget_id FROM widget_tag t0
>>>> WHERE (t0.widget_id = ? AND t0.tag_id IS NULL)]".
>>>>
>>>> In the Widget store if you select a category and then select the empty
>>>> category you get an error page:
>>>> URL:
>>>>
>>>> http://localhost:8080/portal/**app/store/category?categoryId=**
>>>> 0&referringPageId=1(logged<http://localhost:8080/portal/app/store/category?categoryId=0&referringPageId=1(logged>
>>>>
>>>> in as canonical)
>>>> [WARNING] [talledLocalContainer] java.lang.NullPointerException
>>>> [WARNING] [talledLocalContainer] at
>>>>
>>>> org.apache.rave.portal.**service.impl.**DefaultWidgetService.**
>>>> getWidgetsByCategory(**DefaultWidgetService.java:181)
>>>>
>>>>
>>>> The integration tests passed except the last one (OpenID login), but
>>>> that
>>>> one has been fixed in the trunk.
>>>> No real blockers IMO.
>>>>
>>>> Jasha
>>>>
>>>>
>>>> On 25 October 2012 19:12, Chris Geer <ch...@cxtsoftware.com> wrote:
>>>>
>>>> All, I will be merging this over the weekend if there are no
>>>>> objections.
>>>>> (Sunday should be 72 hours). At that time I will do the final reverse
>>>>>
>>>> merge
>>>>
>>>>> into the branch and merge into trunk.
>>>>>
>>>>> Chris
>>>>>
>>>>> On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <ch...@cxtsoftware.com>
>>>>>
>>>> wrote:
>>>>
>>>>>
>>>>> I think we are at a point where the model-split work is far enough
>>>>>>
>>>>> along
>>>>
>>>>> that we should consider merging it soon. I need to finish up
>>>>>>
>>>>> refactoring
>>>>
>>>>> WidgetRating but after that there will be enough critical mass to merge
>>>>>>
>>>>> it
>>>>>
>>>>>> into the baseline IMHO. Prior to that happening it would be good if we
>>>>>>
>>>>> can
>>>>>
>>>>>> get several people to spend a little time and review the changes that
>>>>>>
>>>>> have
>>>>>
>>>>>> been made. Any volunteers?
>>>>>>
>>>>>> The major changes are:
>>>>>> 1) All IDs in the interfaces have been changed from int to String.
>>>>>> This
>>>>>>
>>>>> is
>>>>>
>>>>>> to support a more flexible ID structure in the future with backends
>>>>>>
>>>>> other
>>>>
>>>>> than JPA
>>>>>> 2) The model has been split into several related chunks. The major
>>>>>>
>>>>> chunks
>>>>
>>>>> are Users/People, Widget and other right now. Object relationships
>>>>>>
>>>>> between
>>>>>
>>>>>> those groups have been broken and replaced with IDs. This is to
>>>>>>
>>>>> support a
>>>>
>>>>> more modular backend eventually. Right now they are all still part of
>>>>>>
>>>>> the
>>>>
>>>>> same JPA persistance unit but that is short term.
>>>>>> 3) The widget model has been changed to make Widget the top level
>>>>>>
>>>>> object
>>>>
>>>>> and WidgetComment, WidgetTag & WidgetRating are subordinate objects.
>>>>>>
>>>>> These
>>>>>
>>>>>> changes include removing the widgetID attribute from the subordinate
>>>>>>
>>>>> object
>>>>>
>>>>>> so that they are associated with the Widget they are attached to. We
>>>>>>
>>>>> also
>>>>
>>>>> consolidated the various services and repositories into the
>>>>>> WidgetService/Repository since acting on the subordinate
>>>>>> object independent of the widget isn't ideal.
>>>>>> 4) The link between RegionWidget and Widget has been replaced with
>>>>>> WidgetID. This has required that we populate the JSP attributes with
>>>>>>
>>>>> the
>>>>
>>>>> list of widgets on a page (in the controller).
>>>>>>
>>>>>> There are plenty of other changes we could make so if there are any
>>>>>> you
>>>>>> think are crucial before the merge please point those out but I'm
>>>>>>
>>>>> hoping
>>>>
>>>>> we
>>>>>
>>>>>> can get the major changes into trunk and then iterate through the
>>>>>> rest.
>>>>>>
>>>>>> Thanks,
>>>>>> Chris
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
Re: Model-Split Branch
Posted by Ate Douma <at...@douma.nu>.
Another issue I also experienced but just now properly verified is that
rendering a page (like canonical home page) since the merge has become
excruciating slow.
I haven't spend any time yet investigating why this is, but I just compared
rendering canonical page 1 with the current trunk against an svn snapshot of
just before the merge. Result: ~ 40 sec. vs. 1 sec.
It seems to me something must be completely wrong here and right now the portal
isn't practically usable anymore.
This is worrisome because I think the model-split is an important step and
amounts to a lot of work by Chris and Matt. I truly hope this is just a simple
configuration issue and nothing fundamental.
Anyone else having a similar experience?
Note: scheduled for tomorrow is the (start of the) release Rave 0.17,
and next week is ApacheCon EU, where both Matt and myself will have a Rave
presentation and demo. So I'd like to have a reliable release at least by then.
Ate
On 10/30/2012 05:11 PM, Ate Douma wrote:
> On 10/29/2012 05:11 AM, Chris Geer wrote:
>> Branch is now merged into trunk. The only change I had to make was to
>> the OmdlOutputAdapter class where I had to comment out the @Component
>> annotation. This was because that class now needed a constructor that took
>> several services to function and @Autowire wasn't working due to order of
>> creation issues I believe. The only place I could find it was used in the
>> code was a place it was manually constructed so I think we are fine but I
>> may have missed something. I ran all the unit tests, integration tests and
>> also spot checked a lot of the capabilities.
>
> Hi Chris,
>
> While trying to reconcile these model-split changes against trunk with the
> content-services sandbox, I encountered a few issues, all related to the
> RegionWidget - Widget separation.
> One minor issue is I think a simple merge error but others aspects are more
> structural.
>
> First of all, currently Rave breaks when accessed from a mobile device: mobile
> rendering now throws a JSP rendering exception because the <rave:render-widget/>
> tag has an extra and required widget parameter which hasn't been set (mobile.jsp
> #78).
>
> Another issue is that the Profile page no longer renders (static) widgets
> because the ProfileController doesn't derive and saves the list of widgets in
> the page to the model (as PageController does). I think you did have this in the
> branch but it got lost during the merge back into trunk.
>
> This functionality, retrieving and deriving the widgets from the page region(s)
> RegionWidgets and storing them as a list on the model so they are available
> during rendering, I'm not very happy with. Especially not when it is stored as a
> list. A map (WidgetId->Widget) might be better already so we don't need to
> iterate on the whole list to find back a matching single widget, as for example
> now done in region.tag.
> But maybe an even cleaner alternative would be retrieving a Widget 'on demand'
> through a custom tag itself, similar to the PersonTag does?
> And tags as RegionWidgetTag then can do this internally and then wouldn't need
> the extra widget parameter.
>
> Thoughts?
>
>>
>> After integration I did find one bug
>> RAVE-837<https://issues.apache.org/jira/browse/RAVE-837> that
>> I will try and fix in the next day or so. Please file tickets for any other
>> bugs and we'll squash them.
>>
>> Just remember now, all the IDs in the interfaces are Strings now, not
>> Longs. Longs are still used in the JPA implementation and converted to
>> strings at the interface level.
>>
>> @Jasha - I fixed the category problem prior to merge.
>>
>> Chris
>>
>> On Fri, Oct 26, 2012 at 4:30 AM, Jasha Joachimsthal <ja...@apache.org>wrote:
>>
>>> Hi Chris,
>>>
>>> I did a few tests and found the following things:
>>>
>>> Widget Store shows a log entry about tags:
>>> [INFO] [talledLocalContainer] 465793 ravePersistenceUnit WARN
>>> [http-8080-1] openjpa.Runtime - Supplied user parameters "[widgetId,
>>> tagId]" do not match expected parameters "[widgetId]" for the prepared
>>> query "PreparedQuery: [select t from JpaWidgetTag t where
>>> t.widgetId=:widgetId and t.tagId=:tagId] --> [SELECT t0.entity_id,
>>> t0.created_date, t0.tag_id, t0.user_id, t0.widget_id FROM widget_tag t0
>>> WHERE (t0.widget_id = ? AND t0.tag_id IS NULL)]".
>>>
>>> In the Widget store if you select a category and then select the empty
>>> category you get an error page:
>>> URL:
>>>
>>> http://localhost:8080/portal/app/store/category?categoryId=0&referringPageId=1(logged
>>>
>>> in as canonical)
>>> [WARNING] [talledLocalContainer] java.lang.NullPointerException
>>> [WARNING] [talledLocalContainer] at
>>>
>>> org.apache.rave.portal.service.impl.DefaultWidgetService.getWidgetsByCategory(DefaultWidgetService.java:181)
>>>
>>>
>>> The integration tests passed except the last one (OpenID login), but that
>>> one has been fixed in the trunk.
>>> No real blockers IMO.
>>>
>>> Jasha
>>>
>>>
>>> On 25 October 2012 19:12, Chris Geer <ch...@cxtsoftware.com> wrote:
>>>
>>>> All, I will be merging this over the weekend if there are no objections.
>>>> (Sunday should be 72 hours). At that time I will do the final reverse
>>> merge
>>>> into the branch and merge into trunk.
>>>>
>>>> Chris
>>>>
>>>> On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <ch...@cxtsoftware.com>
>>> wrote:
>>>>
>>>>> I think we are at a point where the model-split work is far enough
>>> along
>>>>> that we should consider merging it soon. I need to finish up
>>> refactoring
>>>>> WidgetRating but after that there will be enough critical mass to merge
>>>> it
>>>>> into the baseline IMHO. Prior to that happening it would be good if we
>>>> can
>>>>> get several people to spend a little time and review the changes that
>>>> have
>>>>> been made. Any volunteers?
>>>>>
>>>>> The major changes are:
>>>>> 1) All IDs in the interfaces have been changed from int to String. This
>>>> is
>>>>> to support a more flexible ID structure in the future with backends
>>> other
>>>>> than JPA
>>>>> 2) The model has been split into several related chunks. The major
>>> chunks
>>>>> are Users/People, Widget and other right now. Object relationships
>>>> between
>>>>> those groups have been broken and replaced with IDs. This is to
>>> support a
>>>>> more modular backend eventually. Right now they are all still part of
>>> the
>>>>> same JPA persistance unit but that is short term.
>>>>> 3) The widget model has been changed to make Widget the top level
>>> object
>>>>> and WidgetComment, WidgetTag & WidgetRating are subordinate objects.
>>>> These
>>>>> changes include removing the widgetID attribute from the subordinate
>>>> object
>>>>> so that they are associated with the Widget they are attached to. We
>>> also
>>>>> consolidated the various services and repositories into the
>>>>> WidgetService/Repository since acting on the subordinate
>>>>> object independent of the widget isn't ideal.
>>>>> 4) The link between RegionWidget and Widget has been replaced with
>>>>> WidgetID. This has required that we populate the JSP attributes with
>>> the
>>>>> list of widgets on a page (in the controller).
>>>>>
>>>>> There are plenty of other changes we could make so if there are any you
>>>>> think are crucial before the merge please point those out but I'm
>>> hoping
>>>> we
>>>>> can get the major changes into trunk and then iterate through the rest.
>>>>>
>>>>> Thanks,
>>>>> Chris
>>>>>
>>>>
>>>
>>
>
Re: Model-Split Branch
Posted by Ate Douma <at...@douma.nu>.
On 10/29/2012 05:11 AM, Chris Geer wrote:
> Branch is now merged into trunk. The only change I had to make was to
> the OmdlOutputAdapter class where I had to comment out the @Component
> annotation. This was because that class now needed a constructor that took
> several services to function and @Autowire wasn't working due to order of
> creation issues I believe. The only place I could find it was used in the
> code was a place it was manually constructed so I think we are fine but I
> may have missed something. I ran all the unit tests, integration tests and
> also spot checked a lot of the capabilities.
Hi Chris,
While trying to reconcile these model-split changes against trunk with the
content-services sandbox, I encountered a few issues, all related to the
RegionWidget - Widget separation.
One minor issue is I think a simple merge error but others aspects are more
structural.
First of all, currently Rave breaks when accessed from a mobile device: mobile
rendering now throws a JSP rendering exception because the <rave:render-widget/>
tag has an extra and required widget parameter which hasn't been set (mobile.jsp
#78).
Another issue is that the Profile page no longer renders (static) widgets
because the ProfileController doesn't derive and saves the list of widgets in
the page to the model (as PageController does). I think you did have this in the
branch but it got lost during the merge back into trunk.
This functionality, retrieving and deriving the widgets from the page region(s)
RegionWidgets and storing them as a list on the model so they are available
during rendering, I'm not very happy with. Especially not when it is stored as a
list. A map (WidgetId->Widget) might be better already so we don't need to
iterate on the whole list to find back a matching single widget, as for example
now done in region.tag.
But maybe an even cleaner alternative would be retrieving a Widget 'on demand'
through a custom tag itself, similar to the PersonTag does?
And tags as RegionWidgetTag then can do this internally and then wouldn't need
the extra widget parameter.
Thoughts?
>
> After integration I did find one bug
> RAVE-837<https://issues.apache.org/jira/browse/RAVE-837> that
> I will try and fix in the next day or so. Please file tickets for any other
> bugs and we'll squash them.
>
> Just remember now, all the IDs in the interfaces are Strings now, not
> Longs. Longs are still used in the JPA implementation and converted to
> strings at the interface level.
>
> @Jasha - I fixed the category problem prior to merge.
>
> Chris
>
> On Fri, Oct 26, 2012 at 4:30 AM, Jasha Joachimsthal <ja...@apache.org>wrote:
>
>> Hi Chris,
>>
>> I did a few tests and found the following things:
>>
>> Widget Store shows a log entry about tags:
>> [INFO] [talledLocalContainer] 465793 ravePersistenceUnit WARN
>> [http-8080-1] openjpa.Runtime - Supplied user parameters "[widgetId,
>> tagId]" do not match expected parameters "[widgetId]" for the prepared
>> query "PreparedQuery: [select t from JpaWidgetTag t where
>> t.widgetId=:widgetId and t.tagId=:tagId] --> [SELECT t0.entity_id,
>> t0.created_date, t0.tag_id, t0.user_id, t0.widget_id FROM widget_tag t0
>> WHERE (t0.widget_id = ? AND t0.tag_id IS NULL)]".
>>
>> In the Widget store if you select a category and then select the empty
>> category you get an error page:
>> URL:
>>
>> http://localhost:8080/portal/app/store/category?categoryId=0&referringPageId=1(logged
>> in as canonical)
>> [WARNING] [talledLocalContainer] java.lang.NullPointerException
>> [WARNING] [talledLocalContainer] at
>>
>> org.apache.rave.portal.service.impl.DefaultWidgetService.getWidgetsByCategory(DefaultWidgetService.java:181)
>>
>> The integration tests passed except the last one (OpenID login), but that
>> one has been fixed in the trunk.
>> No real blockers IMO.
>>
>> Jasha
>>
>>
>> On 25 October 2012 19:12, Chris Geer <ch...@cxtsoftware.com> wrote:
>>
>>> All, I will be merging this over the weekend if there are no objections.
>>> (Sunday should be 72 hours). At that time I will do the final reverse
>> merge
>>> into the branch and merge into trunk.
>>>
>>> Chris
>>>
>>> On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <ch...@cxtsoftware.com>
>> wrote:
>>>
>>>> I think we are at a point where the model-split work is far enough
>> along
>>>> that we should consider merging it soon. I need to finish up
>> refactoring
>>>> WidgetRating but after that there will be enough critical mass to merge
>>> it
>>>> into the baseline IMHO. Prior to that happening it would be good if we
>>> can
>>>> get several people to spend a little time and review the changes that
>>> have
>>>> been made. Any volunteers?
>>>>
>>>> The major changes are:
>>>> 1) All IDs in the interfaces have been changed from int to String. This
>>> is
>>>> to support a more flexible ID structure in the future with backends
>> other
>>>> than JPA
>>>> 2) The model has been split into several related chunks. The major
>> chunks
>>>> are Users/People, Widget and other right now. Object relationships
>>> between
>>>> those groups have been broken and replaced with IDs. This is to
>> support a
>>>> more modular backend eventually. Right now they are all still part of
>> the
>>>> same JPA persistance unit but that is short term.
>>>> 3) The widget model has been changed to make Widget the top level
>> object
>>>> and WidgetComment, WidgetTag & WidgetRating are subordinate objects.
>>> These
>>>> changes include removing the widgetID attribute from the subordinate
>>> object
>>>> so that they are associated with the Widget they are attached to. We
>> also
>>>> consolidated the various services and repositories into the
>>>> WidgetService/Repository since acting on the subordinate
>>>> object independent of the widget isn't ideal.
>>>> 4) The link between RegionWidget and Widget has been replaced with
>>>> WidgetID. This has required that we populate the JSP attributes with
>> the
>>>> list of widgets on a page (in the controller).
>>>>
>>>> There are plenty of other changes we could make so if there are any you
>>>> think are crucial before the merge please point those out but I'm
>> hoping
>>> we
>>>> can get the major changes into trunk and then iterate through the rest.
>>>>
>>>> Thanks,
>>>> Chris
>>>>
>>>
>>
>
Re: Model-Split Branch
Posted by Chris Geer <ch...@cxtsoftware.com>.
Branch is now merged into trunk. The only change I had to make was to
the OmdlOutputAdapter class where I had to comment out the @Component
annotation. This was because that class now needed a constructor that took
several services to function and @Autowire wasn't working due to order of
creation issues I believe. The only place I could find it was used in the
code was a place it was manually constructed so I think we are fine but I
may have missed something. I ran all the unit tests, integration tests and
also spot checked a lot of the capabilities.
After integration I did find one bug
RAVE-837<https://issues.apache.org/jira/browse/RAVE-837> that
I will try and fix in the next day or so. Please file tickets for any other
bugs and we'll squash them.
Just remember now, all the IDs in the interfaces are Strings now, not
Longs. Longs are still used in the JPA implementation and converted to
strings at the interface level.
@Jasha - I fixed the category problem prior to merge.
Chris
On Fri, Oct 26, 2012 at 4:30 AM, Jasha Joachimsthal <ja...@apache.org>wrote:
> Hi Chris,
>
> I did a few tests and found the following things:
>
> Widget Store shows a log entry about tags:
> [INFO] [talledLocalContainer] 465793 ravePersistenceUnit WARN
> [http-8080-1] openjpa.Runtime - Supplied user parameters "[widgetId,
> tagId]" do not match expected parameters "[widgetId]" for the prepared
> query "PreparedQuery: [select t from JpaWidgetTag t where
> t.widgetId=:widgetId and t.tagId=:tagId] --> [SELECT t0.entity_id,
> t0.created_date, t0.tag_id, t0.user_id, t0.widget_id FROM widget_tag t0
> WHERE (t0.widget_id = ? AND t0.tag_id IS NULL)]".
>
> In the Widget store if you select a category and then select the empty
> category you get an error page:
> URL:
>
> http://localhost:8080/portal/app/store/category?categoryId=0&referringPageId=1(logged
> in as canonical)
> [WARNING] [talledLocalContainer] java.lang.NullPointerException
> [WARNING] [talledLocalContainer] at
>
> org.apache.rave.portal.service.impl.DefaultWidgetService.getWidgetsByCategory(DefaultWidgetService.java:181)
>
> The integration tests passed except the last one (OpenID login), but that
> one has been fixed in the trunk.
> No real blockers IMO.
>
> Jasha
>
>
> On 25 October 2012 19:12, Chris Geer <ch...@cxtsoftware.com> wrote:
>
> > All, I will be merging this over the weekend if there are no objections.
> > (Sunday should be 72 hours). At that time I will do the final reverse
> merge
> > into the branch and merge into trunk.
> >
> > Chris
> >
> > On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <ch...@cxtsoftware.com>
> wrote:
> >
> > > I think we are at a point where the model-split work is far enough
> along
> > > that we should consider merging it soon. I need to finish up
> refactoring
> > > WidgetRating but after that there will be enough critical mass to merge
> > it
> > > into the baseline IMHO. Prior to that happening it would be good if we
> > can
> > > get several people to spend a little time and review the changes that
> > have
> > > been made. Any volunteers?
> > >
> > > The major changes are:
> > > 1) All IDs in the interfaces have been changed from int to String. This
> > is
> > > to support a more flexible ID structure in the future with backends
> other
> > > than JPA
> > > 2) The model has been split into several related chunks. The major
> chunks
> > > are Users/People, Widget and other right now. Object relationships
> > between
> > > those groups have been broken and replaced with IDs. This is to
> support a
> > > more modular backend eventually. Right now they are all still part of
> the
> > > same JPA persistance unit but that is short term.
> > > 3) The widget model has been changed to make Widget the top level
> object
> > > and WidgetComment, WidgetTag & WidgetRating are subordinate objects.
> > These
> > > changes include removing the widgetID attribute from the subordinate
> > object
> > > so that they are associated with the Widget they are attached to. We
> also
> > > consolidated the various services and repositories into the
> > > WidgetService/Repository since acting on the subordinate
> > > object independent of the widget isn't ideal.
> > > 4) The link between RegionWidget and Widget has been replaced with
> > > WidgetID. This has required that we populate the JSP attributes with
> the
> > > list of widgets on a page (in the controller).
> > >
> > > There are plenty of other changes we could make so if there are any you
> > > think are crucial before the merge please point those out but I'm
> hoping
> > we
> > > can get the major changes into trunk and then iterate through the rest.
> > >
> > > Thanks,
> > > Chris
> > >
> >
>
Re: Model-Split Branch
Posted by Jasha Joachimsthal <ja...@apache.org>.
Hi Chris,
I did a few tests and found the following things:
Widget Store shows a log entry about tags:
[INFO] [talledLocalContainer] 465793 ravePersistenceUnit WARN
[http-8080-1] openjpa.Runtime - Supplied user parameters "[widgetId,
tagId]" do not match expected parameters "[widgetId]" for the prepared
query "PreparedQuery: [select t from JpaWidgetTag t where
t.widgetId=:widgetId and t.tagId=:tagId] --> [SELECT t0.entity_id,
t0.created_date, t0.tag_id, t0.user_id, t0.widget_id FROM widget_tag t0
WHERE (t0.widget_id = ? AND t0.tag_id IS NULL)]".
In the Widget store if you select a category and then select the empty
category you get an error page:
URL:
http://localhost:8080/portal/app/store/category?categoryId=0&referringPageId=1(logged
in as canonical)
[WARNING] [talledLocalContainer] java.lang.NullPointerException
[WARNING] [talledLocalContainer] at
org.apache.rave.portal.service.impl.DefaultWidgetService.getWidgetsByCategory(DefaultWidgetService.java:181)
The integration tests passed except the last one (OpenID login), but that
one has been fixed in the trunk.
No real blockers IMO.
Jasha
On 25 October 2012 19:12, Chris Geer <ch...@cxtsoftware.com> wrote:
> All, I will be merging this over the weekend if there are no objections.
> (Sunday should be 72 hours). At that time I will do the final reverse merge
> into the branch and merge into trunk.
>
> Chris
>
> On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <ch...@cxtsoftware.com> wrote:
>
> > I think we are at a point where the model-split work is far enough along
> > that we should consider merging it soon. I need to finish up refactoring
> > WidgetRating but after that there will be enough critical mass to merge
> it
> > into the baseline IMHO. Prior to that happening it would be good if we
> can
> > get several people to spend a little time and review the changes that
> have
> > been made. Any volunteers?
> >
> > The major changes are:
> > 1) All IDs in the interfaces have been changed from int to String. This
> is
> > to support a more flexible ID structure in the future with backends other
> > than JPA
> > 2) The model has been split into several related chunks. The major chunks
> > are Users/People, Widget and other right now. Object relationships
> between
> > those groups have been broken and replaced with IDs. This is to support a
> > more modular backend eventually. Right now they are all still part of the
> > same JPA persistance unit but that is short term.
> > 3) The widget model has been changed to make Widget the top level object
> > and WidgetComment, WidgetTag & WidgetRating are subordinate objects.
> These
> > changes include removing the widgetID attribute from the subordinate
> object
> > so that they are associated with the Widget they are attached to. We also
> > consolidated the various services and repositories into the
> > WidgetService/Repository since acting on the subordinate
> > object independent of the widget isn't ideal.
> > 4) The link between RegionWidget and Widget has been replaced with
> > WidgetID. This has required that we populate the JSP attributes with the
> > list of widgets on a page (in the controller).
> >
> > There are plenty of other changes we could make so if there are any you
> > think are crucial before the merge please point those out but I'm hoping
> we
> > can get the major changes into trunk and then iterate through the rest.
> >
> > Thanks,
> > Chris
> >
>
Re: Model-Split Branch
Posted by Chris Geer <ch...@cxtsoftware.com>.
All, I will be merging this over the weekend if there are no objections.
(Sunday should be 72 hours). At that time I will do the final reverse merge
into the branch and merge into trunk.
Chris
On Fri, Oct 12, 2012 at 5:29 PM, Chris Geer <ch...@cxtsoftware.com> wrote:
> I think we are at a point where the model-split work is far enough along
> that we should consider merging it soon. I need to finish up refactoring
> WidgetRating but after that there will be enough critical mass to merge it
> into the baseline IMHO. Prior to that happening it would be good if we can
> get several people to spend a little time and review the changes that have
> been made. Any volunteers?
>
> The major changes are:
> 1) All IDs in the interfaces have been changed from int to String. This is
> to support a more flexible ID structure in the future with backends other
> than JPA
> 2) The model has been split into several related chunks. The major chunks
> are Users/People, Widget and other right now. Object relationships between
> those groups have been broken and replaced with IDs. This is to support a
> more modular backend eventually. Right now they are all still part of the
> same JPA persistance unit but that is short term.
> 3) The widget model has been changed to make Widget the top level object
> and WidgetComment, WidgetTag & WidgetRating are subordinate objects. These
> changes include removing the widgetID attribute from the subordinate object
> so that they are associated with the Widget they are attached to. We also
> consolidated the various services and repositories into the
> WidgetService/Repository since acting on the subordinate
> object independent of the widget isn't ideal.
> 4) The link between RegionWidget and Widget has been replaced with
> WidgetID. This has required that we populate the JSP attributes with the
> list of widgets on a page (in the controller).
>
> There are plenty of other changes we could make so if there are any you
> think are crucial before the merge please point those out but I'm hoping we
> can get the major changes into trunk and then iterate through the rest.
>
> Thanks,
> Chris
>