[dev] Horde_Share_Base::listShares()

[Michael Rubinsky]

Quoting Jan Schneider <jan at horde.org>:

> Zitat von Gunnar Wrobel <wrobel at horde.org>:
>
>> Hi!
>>
>> The function mentioned above does support several sorting/paging  
>> features ('from', 'count', 'sort_by', 'direction'). As far as I can  
>> tell none of these parameters are actively used within our code  
>> base (with the exception of 'sort_by' but 'name' seems to be the  
>> only setting used here). Currently only the Sql-Drivers supports  
>> these features and I wonder if we really need them.

Huh. That's embarrasing. I'm pretty sure it worked at one point, but  
maybe that was back in the DT days. I never realised that the paging  
parameters were ignored by the SQL share drivers.  I know that Ansel  
passes these values when performing certain searches, like listing all  
of a user's galleries, or most recent galleries, or any number of  
other searches. The View code in Ansel, however, also honors the  
per-page prefs, which is probably why I never noticed this.

Ansel could potentially generate a large number of shares, since each  
gallery/subgallery is a separate share - though I imagine it would be  
rare to reach the thousands per user number. I can understand the  
issue of large session caches in certain cases.

I have mixed feelings about not implementing this though. On the one  
hand, if the user *does* have a large number of shares - and I'm  
honestly not sure how large that number would have to be to start  
beccoming an issue - then it seems a waste to carry around potentially  
the user's entire share list in the session if all that was ever  
needed were the first 9 shares. For example - imagine I have 100 Ansel  
galleries. I log into Horde, and since Ansel is my initial  
application, I load the first page of my gallery list, I then move on  
to reading email and don't go back to Ansel again during this session.  
  Without support for paging, this would cause all 100 of my ansel  
shares to be carried around for the life of my session...and from what  
you are saying this is currently happening.

So I guess the question is: Is it more expensive to keep more search  
result entries in the list cache - each with a smaller subset of  
shares, or to keep a smaller number of search results, but each with  
larger numbers of shares?

I think the case where a user would load thousands of shares, then  
sort them in multiple ways is a fringe case. More likely, a smaller  
subset of those shares would be loaded at a time e.g., when paging  
through a gallery list.

Another option is to ignore the paging/sorting parameters when caching  
and hitting the DB, and apply to paging limits in PHP, this has the  
disadvantage of having to apply the sort to the share list each time,  
prior to applying the limits. Since these parameters are not currently  
being passed to the DB anyway, this is probably what we are already  
doing in the application code.

>> Horde_Share_Base::listShares() is fully replaced within  
>> Horde_Share_Sql and Horde_Share_Sql_Hierarchical.

Doing it this way is probably historical cruft, left over from when  
these drivers were written, and DT was the only supported driver.


>> For Horde_Share_Kolab and Horde_Share_Datatree  
>> Horde_Share_Base::listShares() is called which delegates to  
>> Horde_Share_{Datatree,Kolab}::_listShares(). The sorting/paging  
>> apparently should be handled within Horde_Share_Base::listShares()  
>> for these drivers. The current implementation is incomplete however.

Now that DT is deprecated, if we decide to implement the paging in the  
DB, we could probably just make Horde_Share_Base::listShares()  
abstract. Otherwise, yeah, we could just implement it in  
Horde_Share_Base::listShares()

>> I don't mind completing the implementation if necessary but there  
>> are some things that currently puzzle me. Assuming a user has a few  
>> thousand shares and we would use the paging feature for example:  
>> Wouldn't the current Horde_Share_Sql:listShares() quickly fill the  
>> Horde_Share_Base::_listcache with plenty of caching information  
>> that gets stored in the session? For every sort setting it would be  
>> the same thing.

This is assuming that all thousand shares would be returned in each  
query. If we implement paging, it's likely only a small subset of  
those shares would be returned for any one query - the amount that  
would fit on a page. The exception to this is when querying for things  
like select lists, but those would likely not be requested with more  
than a single sort parameter.  Generally, though, yes, each sort/page  
combmination would be cached separately.

>> I also wonder a bit if that is actually information that should go  
>> into the session at all.

I know that in FW_3, at least at one time, it greatly improved  
performance since share queries tend to be expensive, especially on  
mysql. Jan could probably answer this part better than I, since he has  
done some benchmarking recently.

>> On the other hand there has been a lot of work put into the Share  
>> driver recently and as far as I understand it there is more to  
>> come. I just wanted to get some feedback to understand where this  
>> is heading and if I should ignore the topic for now.
>
> If we don't use some of the paging/sorting features and don't come  
> up with any use case for it (and honestly, who is going to have such  
> a huge load of shares?), I'm fine with removing them to reduce  
> complexity.

Honestly not 100% sure which way I'd prefer, but I think I'm leaning  
towards having the DB honor the parameters, instead of having to  
sort/page only in pure php.


Thanks,
mike

--
The Horde Project (www.horde.org)
mrubinsk at horde.org

"Time just hates me. That's why it made me an adult." - Josh Joplin