You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@roller.apache.org by sn...@apache.org on 2007/07/26 16:37:08 UTC
svn commit: r559835 - in /roller/trunk/apps/weblogger:
src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java
web/WEB-INF/velocity/weblog.vm
Author: snoopdave
Date: Thu Jul 26 07:37:07 2007
New Revision: 559835
URL: http://svn.apache.org/viewvc?view=rev&rev=559835
Log:
Fix for ROL-548 - Bookmark display macro not obeying sort order
Modified:
roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java
roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm
Modified: roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java
URL: http://svn.apache.org/viewvc/roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java?view=diff&rev=559835&r1=559834&r2=559835
==============================================================================
--- roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java (original)
+++ roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java Thu Jul 26 07:37:07 2007
@@ -22,7 +22,9 @@
import java.util.Iterator;
import java.util.List;
import java.util.Set;
+import java.util.TreeSet;
import org.apache.roller.weblogger.WebloggerException;
+import org.apache.roller.weblogger.pojos.BookmarkComparator;
import org.apache.roller.weblogger.pojos.WeblogBookmark;
import org.apache.roller.weblogger.pojos.WeblogBookmarkFolder;
@@ -115,7 +117,24 @@
return wrappedCollection;
}
-
+ public List getBookmarksSorted() {
+ TreeSet initialCollection = new TreeSet(new BookmarkComparator());
+ initialCollection.addAll(this.pojo.getBookmarks());
+
+ // iterate through and wrap
+ // we force the use of an ArrayList because it should be good enough to cover
+ // for any Collection type we encounter.
+ ArrayList wrappedCollection = new ArrayList(initialCollection.size());
+ Iterator it = initialCollection.iterator();
+ int i = 0;
+ while(it.hasNext()) {
+ wrappedCollection.add(i,WeblogBookmarkWrapper.wrap((WeblogBookmark) it.next()));
+ i++;
+ }
+
+ return wrappedCollection;
+ }
+
public List retrieveBookmarks(boolean subfolders)
throws WebloggerException {
Modified: roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm
URL: http://svn.apache.org/viewvc/roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm?view=diff&rev=559835&r1=559834&r2=559835
==============================================================================
--- roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm (original)
+++ roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm Thu Jul 26 07:37:07 2007
@@ -366,7 +366,7 @@
*#
#macro(_showBookmarkLinksList $folderObject $subfolders $expanding )
#if ($expanding) #_showCommonJavascript() #end
- #set($bookmarks = $folderObject.getBookmarks())
+ #set($bookmarks = $folderObject.getBookmarksSorted())
#set($folders = $folderObject.getFolders())
#set($divId = $utils.replace($folderObject.name, " ", "_" ))
#if ($folderObject.name != "root" && $expanding && $subfolders && ($folderObject.getBookmarks().size() > 0 || $folderObject.getFolders().size() > 0))
Re: svn commit: r559835 - in /roller/trunk/apps/weblogger: src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java web/WEB-INF/velocity/weblog.vm
Posted by Dave <sn...@gmail.com>.
On 7/26/07, Allen Gilliland <al...@sun.com> wrote:
>
>
> Dave wrote:
> > On 7/26/07, Allen Gilliland <al...@sun.com> wrote:
> >> Is there a reason why we had to do this as a new method? Wouldn't we
> >> want folder.getBookmarks() to just return the sorted collection?
> >
> > I didn't want to muck with the getBookmarks() method because it is an
> > ORM managed relationship, i.e. JPA fetches the bookmarks. Think we
> > should manage that relationship instead so we can have one method?
>
>
> Hmm, I think the best thing is to leave the relationship as managed by
> the ORM tool, but configure things so that the collection comes out
> sorted. I think that should be doable, although it's a little more
> complicated with bookmarks because they are sorted by a separate
> priority field.
>
> If we can't get the ORM tool to properly sort things then I think the
> next best option is to just have the pojo wrapper method do the sorting.
> I don't see any reason why folderWrapper.getBookmarks() can't start by
> getting the collection via the real pojo, then sort the collection, then
> wrap. Or alternatively the order could be to get the collection via the
> real pojo, wrap them, then sort.
The folder wrapper's getBookmarks() method now does the sorting, so
now there is no new getBookmarksSorted() method. If we figure out how
to get a sorted collection back from JPA via that one-to-many
relationship, the change will be transparent.
- Dave
Re: svn commit: r559835 - in /roller/trunk/apps/weblogger:
src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java
web/WEB-INF/velocity/weblog.vm
Posted by Allen Gilliland <al...@sun.com>.
Dave wrote:
> On 7/26/07, Allen Gilliland <al...@sun.com> wrote:
>> Is there a reason why we had to do this as a new method? Wouldn't we
>> want folder.getBookmarks() to just return the sorted collection?
>
> I didn't want to muck with the getBookmarks() method because it is an
> ORM managed relationship, i.e. JPA fetches the bookmarks. Think we
> should manage that relationship instead so we can have one method?
Hmm, I think the best thing is to leave the relationship as managed by
the ORM tool, but configure things so that the collection comes out
sorted. I think that should be doable, although it's a little more
complicated with bookmarks because they are sorted by a separate
priority field.
If we can't get the ORM tool to properly sort things then I think the
next best option is to just have the pojo wrapper method do the sorting.
I don't see any reason why folderWrapper.getBookmarks() can't start by
getting the collection via the real pojo, then sort the collection, then
wrap. Or alternatively the order could be to get the collection via the
real pojo, wrap them, then sort.
The nice thing about having the pojo wrappers static now is that we can
actually add custom logic into the wrapped methods so that they make
little tweaks which are appropriate for rendering. I am already doing
this in various places in other pojos to do things like html escape
content, build urls, choose between 2 ways of returning the data, etc.
So if we can't get the folder pojo to actually return the bookmarks in
sorted order then I think we should definitely put the sorting logic in
the folderWrapper methods.
-- Allen
>
> - Dave
>
>
>
>
>>
>> -- Allen
>>
>>
>> snoopdave@apache.org wrote:
>> > Author: snoopdave
>> > Date: Thu Jul 26 07:37:07 2007
>> > New Revision: 559835
>> >
>> > URL: http://svn.apache.org/viewvc?view=rev&rev=559835
>> > Log:
>> > Fix for ROL-548 - Bookmark display macro not obeying sort order
>> >
>> > Modified:
>> >
>> roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java
>>
>> > roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm
>> >
>> > Modified:
>> roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java
>>
>> > URL:
>> http://svn.apache.org/viewvc/roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java?view=diff&rev=559835&r1=559834&r2=559835
>>
>> >
>> ==============================================================================
>>
>> > ---
>> roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java
>> (original)
>> > +++
>> roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java
>> Thu Jul 26 07:37:07 2007
>> > @@ -22,7 +22,9 @@
>> > import java.util.Iterator;
>> > import java.util.List;
>> > import java.util.Set;
>> > +import java.util.TreeSet;
>> > import org.apache.roller.weblogger.WebloggerException;
>> > +import org.apache.roller.weblogger.pojos.BookmarkComparator;
>> > import org.apache.roller.weblogger.pojos.WeblogBookmark;
>> > import org.apache.roller.weblogger.pojos.WeblogBookmarkFolder;
>> >
>> > @@ -115,7 +117,24 @@
>> > return wrappedCollection;
>> > }
>> >
>> > -
>> > + public List getBookmarksSorted() {
>> > + TreeSet initialCollection = new TreeSet(new
>> BookmarkComparator());
>> > + initialCollection.addAll(this.pojo.getBookmarks());
>> > +
>> > + // iterate through and wrap
>> > + // we force the use of an ArrayList because it should be
>> good enough to cover
>> > + // for any Collection type we encounter.
>> > + ArrayList wrappedCollection = new
>> ArrayList(initialCollection.size());
>> > + Iterator it = initialCollection.iterator();
>> > + int i = 0;
>> > + while(it.hasNext()) {
>> > +
>> wrappedCollection.add(i,WeblogBookmarkWrapper.wrap((WeblogBookmark)
>> it.next()));
>> > + i++;
>> > + }
>> > +
>> > + return wrappedCollection;
>> > + }
>> > +
>> > public List retrieveBookmarks(boolean subfolders)
>> > throws WebloggerException {
>> >
>> >
>> > Modified: roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm
>> > URL:
>> http://svn.apache.org/viewvc/roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm?view=diff&rev=559835&r1=559834&r2=559835
>>
>> >
>> ==============================================================================
>>
>> > --- roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm
>> (original)
>> > +++ roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm Thu
>> Jul 26 07:37:07 2007
>> > @@ -366,7 +366,7 @@
>> > *#
>> > #macro(_showBookmarkLinksList $folderObject $subfolders $expanding )
>> > #if ($expanding) #_showCommonJavascript() #end
>> > - #set($bookmarks = $folderObject.getBookmarks())
>> > + #set($bookmarks = $folderObject.getBookmarksSorted())
>> > #set($folders = $folderObject.getFolders())
>> > #set($divId = $utils.replace($folderObject.name, " ", "_" ))
>> > #if ($folderObject.name != "root" && $expanding && $subfolders
>> && ($folderObject.getBookmarks().size() > 0 ||
>> $folderObject.getFolders().size() > 0))
>> >
>> >
>>
Re: svn commit: r559835 - in /roller/trunk/apps/weblogger: src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java web/WEB-INF/velocity/weblog.vm
Posted by Dave <sn...@gmail.com>.
On 7/26/07, Allen Gilliland <al...@sun.com> wrote:
> Is there a reason why we had to do this as a new method? Wouldn't we
> want folder.getBookmarks() to just return the sorted collection?
I didn't want to muck with the getBookmarks() method because it is an
ORM managed relationship, i.e. JPA fetches the bookmarks. Think we
should manage that relationship instead so we can have one method?
- Dave
>
> -- Allen
>
>
> snoopdave@apache.org wrote:
> > Author: snoopdave
> > Date: Thu Jul 26 07:37:07 2007
> > New Revision: 559835
> >
> > URL: http://svn.apache.org/viewvc?view=rev&rev=559835
> > Log:
> > Fix for ROL-548 - Bookmark display macro not obeying sort order
> >
> > Modified:
> > roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java
> > roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm
> >
> > Modified: roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java
> > URL: http://svn.apache.org/viewvc/roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java?view=diff&rev=559835&r1=559834&r2=559835
> > ==============================================================================
> > --- roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java (original)
> > +++ roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java Thu Jul 26 07:37:07 2007
> > @@ -22,7 +22,9 @@
> > import java.util.Iterator;
> > import java.util.List;
> > import java.util.Set;
> > +import java.util.TreeSet;
> > import org.apache.roller.weblogger.WebloggerException;
> > +import org.apache.roller.weblogger.pojos.BookmarkComparator;
> > import org.apache.roller.weblogger.pojos.WeblogBookmark;
> > import org.apache.roller.weblogger.pojos.WeblogBookmarkFolder;
> >
> > @@ -115,7 +117,24 @@
> > return wrappedCollection;
> > }
> >
> > -
> > + public List getBookmarksSorted() {
> > + TreeSet initialCollection = new TreeSet(new BookmarkComparator());
> > + initialCollection.addAll(this.pojo.getBookmarks());
> > +
> > + // iterate through and wrap
> > + // we force the use of an ArrayList because it should be good enough to cover
> > + // for any Collection type we encounter.
> > + ArrayList wrappedCollection = new ArrayList(initialCollection.size());
> > + Iterator it = initialCollection.iterator();
> > + int i = 0;
> > + while(it.hasNext()) {
> > + wrappedCollection.add(i,WeblogBookmarkWrapper.wrap((WeblogBookmark) it.next()));
> > + i++;
> > + }
> > +
> > + return wrappedCollection;
> > + }
> > +
> > public List retrieveBookmarks(boolean subfolders)
> > throws WebloggerException {
> >
> >
> > Modified: roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm
> > URL: http://svn.apache.org/viewvc/roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm?view=diff&rev=559835&r1=559834&r2=559835
> > ==============================================================================
> > --- roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm (original)
> > +++ roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm Thu Jul 26 07:37:07 2007
> > @@ -366,7 +366,7 @@
> > *#
> > #macro(_showBookmarkLinksList $folderObject $subfolders $expanding )
> > #if ($expanding) #_showCommonJavascript() #end
> > - #set($bookmarks = $folderObject.getBookmarks())
> > + #set($bookmarks = $folderObject.getBookmarksSorted())
> > #set($folders = $folderObject.getFolders())
> > #set($divId = $utils.replace($folderObject.name, " ", "_" ))
> > #if ($folderObject.name != "root" && $expanding && $subfolders && ($folderObject.getBookmarks().size() > 0 || $folderObject.getFolders().size() > 0))
> >
> >
>
Re: svn commit: r559835 - in /roller/trunk/apps/weblogger:
src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java
web/WEB-INF/velocity/weblog.vm
Posted by Allen Gilliland <al...@sun.com>.
Is there a reason why we had to do this as a new method? Wouldn't we
want folder.getBookmarks() to just return the sorted collection?
-- Allen
snoopdave@apache.org wrote:
> Author: snoopdave
> Date: Thu Jul 26 07:37:07 2007
> New Revision: 559835
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=559835
> Log:
> Fix for ROL-548 - Bookmark display macro not obeying sort order
>
> Modified:
> roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java
> roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm
>
> Modified: roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java
> URL: http://svn.apache.org/viewvc/roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java?view=diff&rev=559835&r1=559834&r2=559835
> ==============================================================================
> --- roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java (original)
> +++ roller/trunk/apps/weblogger/src/java/org/apache/roller/weblogger/pojos/wrapper/WeblogBookmarkFolderWrapper.java Thu Jul 26 07:37:07 2007
> @@ -22,7 +22,9 @@
> import java.util.Iterator;
> import java.util.List;
> import java.util.Set;
> +import java.util.TreeSet;
> import org.apache.roller.weblogger.WebloggerException;
> +import org.apache.roller.weblogger.pojos.BookmarkComparator;
> import org.apache.roller.weblogger.pojos.WeblogBookmark;
> import org.apache.roller.weblogger.pojos.WeblogBookmarkFolder;
>
> @@ -115,7 +117,24 @@
> return wrappedCollection;
> }
>
> -
> + public List getBookmarksSorted() {
> + TreeSet initialCollection = new TreeSet(new BookmarkComparator());
> + initialCollection.addAll(this.pojo.getBookmarks());
> +
> + // iterate through and wrap
> + // we force the use of an ArrayList because it should be good enough to cover
> + // for any Collection type we encounter.
> + ArrayList wrappedCollection = new ArrayList(initialCollection.size());
> + Iterator it = initialCollection.iterator();
> + int i = 0;
> + while(it.hasNext()) {
> + wrappedCollection.add(i,WeblogBookmarkWrapper.wrap((WeblogBookmark) it.next()));
> + i++;
> + }
> +
> + return wrappedCollection;
> + }
> +
> public List retrieveBookmarks(boolean subfolders)
> throws WebloggerException {
>
>
> Modified: roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm
> URL: http://svn.apache.org/viewvc/roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm?view=diff&rev=559835&r1=559834&r2=559835
> ==============================================================================
> --- roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm (original)
> +++ roller/trunk/apps/weblogger/web/WEB-INF/velocity/weblog.vm Thu Jul 26 07:37:07 2007
> @@ -366,7 +366,7 @@
> *#
> #macro(_showBookmarkLinksList $folderObject $subfolders $expanding )
> #if ($expanding) #_showCommonJavascript() #end
> - #set($bookmarks = $folderObject.getBookmarks())
> + #set($bookmarks = $folderObject.getBookmarksSorted())
> #set($folders = $folderObject.getFolders())
> #set($divId = $utils.replace($folderObject.name, " ", "_" ))
> #if ($folderObject.name != "root" && $expanding && $subfolders && ($folderObject.getBookmarks().size() > 0 || $folderObject.getFolders().size() > 0))
>
>