[dev] Horde_Share_Base::listShares()

[Michael Rubinsky]

Quoting Jan Schneider <jan at horde.org>:

> Zitat von Michael Rubinsky <mrubinsk at horde.org>:
>
>>
>> Quoting Michael Rubinsky <mrubinsk at horde.org>:
>>
>>> Quoting Michael Rubinsky <mrubinsk at horde.org>:
>>>
>>>> Quoting Gunnar Wrobel <wrobel at horde.org>:
>>>>
>>>>>
>>>>> Zitat von Jan Schneider <jan at horde.org>:
>>>>>
>>>>>> Zitat von Michael Rubinsky <mrubinsk at horde.org>:
>>>>>>
>>>>>>> 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.
>>>>>
>>>>> They don't. As mentioned in my original mail the SQL drivers are  
>>>>> the only ones that actually support it. It does not work within  
>>>>> the Horde_Share_Base::listShares() but that only applies to the  
>>>>> Datatree and Kolab drivers.
>>>>
>>>> Ah. Ok. I apologize, I misunderstood what you were saying, and I  
>>>> don't have access to the code at the moment to look. At least now  
>>>> I don't think I'm losing my mind :)
>>>>
>>>>
>>>>> Two additional questions though:
>>>>>
>>>>> 1) Why do we have a separate Sql and Sql_Hierarchical driver?  
>>>>> Wouldn't it make sense to collapse the two and always be  
>>>>> hierarchical? I'd have no problems supporting a tree structure  
>>>>> with the Kolab driver. In fact I'd like to do that.
>>>>
>>>> Originally, when hierarchical support was removed from DT for  
>>>> performance reasons, Ansel had kept it's own share driver, then  
>>>> when Duck started work on the SQL share driver, we kept the  
>>>> hierarchical driver seperate both for performance, and to keep  
>>>> the SQL driver as simple as possible, since none of the other  
>>>> apps need hierarchical support.
>>>>
>>>> There are some other differences, there are no share_names in the  
>>>> hierarchical driver; share_ids are used for everything. There is  
>>>> no need for the names, and causes confusion (though that's  
>>>> probably just my feeling based on bad experiences with this while  
>>>> adding initial share support to Turba - the code has  
>>>> significantly improved since then). I also don't want Ansel's  
>>>> galleries to have to be identified by these string hashes, it  
>>>> would break existing permalinks, makes for long ugly urls, and  
>>>> would probably require some refactoring in Ansel...not that  
>>>> refactoring is "bad", just sayin'...
>>>>
>>>> If it's possible to combine them without complicating things too  
>>>> much, degrading performance, and keeping the above mentioned  
>>>> points about share_name vs share_id intact, that's fine with me.
>>>
>>> Ok. Now that I've spend some more time with the code, the only  
>>> remaining reason (beside any potential performance issues related  
>>> to the longer share_parent fields) to really keep them separate  
>>> would be to maintain using share_ids as identifiers in Ansel. If  
>>> we combine the drivers, we'd need to use the share_names to  
>>> identify the galleries, which would lead to existing links to  
>>> galleries being broken.  We'd also obviously need to run some data  
>>> conversions on all the ansel tables - and the gallery key image  
>>> code would need some refactoring (since it uses negative  
>>> gallery-ids to indicate they are key images).
>>
>> ...and to contradict myself again, I actually think it would be  
>> possible to keep using the gallery ids. I'll try to work this out  
>> on a separate branch and see where it goes.
>
> I buy the argument for shorter URLs, but non-guessable URLs for  
> direct guest access have their merits too.

True. Though, the only time this would truly protect the gallery is if  
it's a child of a private gallery, since the user could find the  
gallery via Ansel's UI anyway.

That being said, my biggest concern is maintaining existing gallery  
links that may be out in the wild. This would break all links, and  
galleries embedded on other sites/blogs.  If we're willing to say it's  
ok to break this with the 2.0 release, then ok, let's do it.


mike

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