Re: set_display_context_callback() and incorrect pagination

Maybe I should summarize the problem in two lines of code rather than
too many lines of text :)

GD's paginator.html.php contains the following snippet:

case "item":
       if (isset($item)):
         $parent = $item->parent();
       endif;
...
       if (isset($parent)):
         $siblings = $parent->children();
         for ($i = 1; $i <= $total; $i++):
           $_pagelist[$i] = $siblings[$i-1]->url();
         endfor;
       endif;

Now, there are better ways to get the values we need (the reason they
fetch _every_ child is to show pagination of the form "1 2 ... 12 13
*14* 15 16 17 ...  205 206"; we can obviously optimize that).

But the problem is that $total (which comes from sibling_count) is NOT
the same as the number of elements in $item->parent()->children() .

Is there a better API to use to get the "siblings" (cousins, perhaps)
rather than asking for the parent's children?

-- Mike

On Mon, Jan 28, 2013 at 6:54 PM, Mike Miller <[email protected]> wrote:
> On Mon, Jan 28, 2013 at 6:02 PM, Bharat Mediratta <[email protected]> wrote:
>>
>> I'm trying to work through what you're saying.  It sounds like the
>> latestupdates module is creating an album that's huge (11k siblings?)
>
> Yes; it lists _everything_ in my gallery.
>
>> and
>> the Grey Dragon theme is trying to do an operation on every sibling when it
>> does a page render.  So then theoretically GD can't handle any really large
>> album (and it'll get slower as albums get larger).  Does that sound right?
>
> Probably, but the inability to scale is not the main problem. I'll
> rewrite below.
>
>> I'm not 100% happy with the display context code.  As designed, it allows us
>> to keep track of logical groupings of items (search results, tags, etc) as
>> separare from the "physical" groupings (albums).
>
> Which makes sense, as the user may also want to have a "Next" /
> "Previous" button that operate according to the virtual album (search
> results) and not necessarily only via the album (or a custom
> controller + view like tag / tag_album)
>
>> That part is fine.  The
>> part that's messy is that we store the context in your session so that we
>> don't have to clutter the url with it. That part I don't like because it
>> means that there's a hidden piece of information that governs what the
>> resulting UI is going to look like and that hidden piece of information
>> propagates separately from the URL.  It's caused us problems in the past -
>> like when you click on a random image from a search results page it tries to
>> show that random image in the context of your search results.
>
> And this is where my problem begins. The problem is NOT that GD is
> trying to operate on all images; that's slow and ugly, but bearable.
> The problem is that on one hand, GD tries to operate on the
> sibling_count it got from the context (which indicates that it came
> from a view of all items) but since the URL is just a regular
> g3/album/subalbum/item, there AREN'T actually 11K siblings; there are
> only, say, 15.
>
> If the photo links from latestupdates where of the form:
>
> /g3/latestupdates/view_photo?full_path=album/subalbum/item (or
> something fancier using URL Rewriting)
>
> then it would make sense for the user to click on "Next" and get the
> next most recent item, not the next item in the same album as the
> current image.
>
> But they're not.
>
> Instead, the user sees in the browser
>
> /g3/album/subalbum/item
>
> with no indication that he came to this link from latestitems. As
> such, he should see the same thing as if he manually typed in the URL.
>
> In order to do this, the display context cannot be cached (or at least
> has to be reset somehow)
>
>> I'm in favor of actually creating a new URL path per context to resolve
>> this, but if I recall correctly there were problems with doing that.  I
>> can't remember what they were, though - I'd have to investigate.  If
>> somebody would like to take a shot at this, I'd be happy to help.
>>
>> On the crashing problem - this is a GD issue.  It's not a good idea to
>> attempt to do operations on an open-ended number of siblings.  It's
>> eventually going to hit a resource limit.  GD needs to restrain itself in
>> some fashion...
>
> Agreed. But it's a problem in the default Wind as well, although there
> it's just cosmetic. In Wind, I see "XXX of <TOTAL IN GALLERY>" on the
> top, whereas if I'd gone to the URL directly, I see "XXX of <TOTAL IN
> ALBUM>".
>
> When I commented out the storage of the display context (as mentioned
> below), I saw no adverse effects. Did I break anything without
> realizing it?
>
>> -Bharat
>>
>>
>> On Sat, Jan 26, 2013 at 11:40 AM, Mike Miller <[email protected]> wrote:
>>>
>>> Hi All,
>>>
>>> I think I've identified a bug (mostly cosmetic in Wind, fatal in Grey
>>> Dragon), but I'm not sure how to fix it.
>>>
>>> The original symptom I noticed (in Grey Dragon) is that if I use
>>> either the dynamic or the latestalbums modules to see recent items,
>>> and then click on one of the images, the page aborts loading right
>>> after the heading. If I click on the parent album (using a breadcrumb
>>> or by typing a URL manually), and then try to return to the photo's
>>> page again, it loads properly.
>>>
>>> A quick investigation revealed that GD's paginator is trying to load
>>> each sibling, and although sibling_count is 11912 for me (the number
>>> of items in my gallery), it fails to retrieve a specific sibling
>>> (corresponding to the the number of items in the album (i.e., the
>>> actual sibling count) plus 1). It seems that when the items are loaded
>>> via latestupdates or dynamic, a call to set_display_context_callback()
>>> is made, and this sets the function that returns the "global variable"
>>> $total which is based on sibling_count (in
>>> modules/gallery/libraries/Gallery_View.php). If I examine the
>>> g3_caches table, I can confirm that when viewing the photo page, the
>>> callback is set to something like
>>> latestupdates_Controller::get_display_context. Opening the album page
>>> resets this (in modules/gallery/controllers/albums.php) and then the
>>> count is set correctly.
>>>
>>> In Wind, it's less severe; the paginator doesn't try to retrieve
>>> information about the sibling, so the only effect is that despite the
>>> fact that the URL is /path/to/actual/album/name/PHOTO, the paginator
>>> says "XXX of <TOTAL NUMBER>" rather than "XXX of <ITEMS IN ALBUM>"
>>>
>>> If this was a G2 like setup where I was accessing a URL as
>>> /latestupdates/PHOTO_NAME instead of /album/name/PHOTO_NAME, I could
>>> understand the need to show it as part of the virtual album. But in
>>> G3, once I've clicked to a full URL, the fact that I was once upon a
>>> time in /latestupdates/ presumably shouldn't matter; there's no
>>> indication to the user who see /album/name/PHOTO_NAME in his browser
>>> that the information is being calculated as though he's paging through
>>> latestupdates's listing.
>>>
>>> That said, without disabling the call to Cache::instance()->set() in
>>> set_display_context_callback (in modules/gallery/helpers/item.php),
>>> I'm not sure how to fix this.
>>>
>>> Any ideas?
>>>
>>> -- Mike
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
>>> MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
>>> with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
>>> MVPs and experts. ON SALE this month only -- learn more at:
>>> http://p.sf.net/sfu/learnnow-d2d
>>> __[ g a l l e r y - d e v e l ]_________________________
>>>
>>> [ list info/archive --> http://gallery.sf.net/lists.php ]
>>> [ gallery info/FAQ/download --> http://gallery.sf.net ]
>>>
>>

------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnnow-d2d
__[ g a l l e r y - d e v e l ]_________________________

[ list info/archive --> http://gallery.sf.net/lists.php ]
[ gallery info/FAQ/download --> http://gallery.sf.net ]