On 19 Apr 2016, at 14:23, Thomas Mortagne
<thomas.mortagne(a)xwiki.com> wrote:
On Tue, Apr 19, 2016 at 1:14 PM, Vincent Massol <vincent(a)massol.net> wrote:
On 19 Apr 2016, at 12:47, Thomas Mortagne
<thomas.mortagne(a)xwiki.com> wrote:
On Tue, Apr 19, 2016 at 10:28 AM, Vincent Massol <vincent(a)massol.net> wrote:
> On 19 Apr 2016, at 10:25, Vincent Massol <vincent(a)massol.net> wrote:
>
>
>> On 19 Apr 2016, at 10:07, Thomas Mortagne <thomas.mortagne(a)xwiki.com>
wrote:
>>
>> Like Edy, I'm not a big fan of the forced document based entry point
>> since it might not makes any sense for some use cases.
>
> As I mentioned in my previous mail we don’t need to consider it a reference to a
document. We could just make it a reference to anything. The module using the tmp service
would know what type of reference it is and it could cast it to a document reference if it
knows it’s that.
Actually this wouldn’t work since it would have one effect: we wouldn’t be able to check
rights in the Tmp Handler which is a problem…
Yes, which is why I said in my previous mail that we can't really get rid of it.
We could decide to use a serialized Resource Reference and develop a Rights Checker
accepting any Resource Reference (we would only implement check for Entity Resource
Reference to start with). This means also inventing a serialization format for Resource
References (something with a prefix representing the type as with Link Resource
References).
Since this is a bit complex we could start with an Entity Reference for now and implement
this later on.
Extending what I started to express in my previous mail: IMO we should
stop trying to make /tmp/ the ultimate generic temporary resource
provider, we can live with something dedicated to providing a
filesystem file associated to an entity and test access based on this
entity. Any module that have another use case can easily bypass /tmp/
and provide its own resource reference handler. Making /tmp/ super
generic just ends up reinventig a new (and more complex from what I
can see in the proposals) resource reference handler framework. I'm
even removing my comment about empty entity.
That means:
http://<server>/<context>/tmp/<entitytype>:<entity
reference>/<module-dependent resource path>
It's easy to support any resource reference instead of the entity
reference later just by changing the meaning of <entitytype> into
<resourcetype> so we don't really need to deal with it now.
Thanks for the reply Thomas.
How do we parse "<entitytype>:<entity reference>”? AFAIK we don’t have a
parser for this.
I’d prefer to have <serialized document reference> FTM since we have a parser for
that and change that later on if need be. My goal was to implement quickly the tmp handler
since I judged it was going to take about the same time as fixing the existing TmpAction
to support Nested Spaces. This thread is starting to prove I was wrong and I’m very
tempted to drop the topic and to go back to just fixing TempAction. That would be a pity
IMO. (I’ve already spent a lot more time than I had planned with the SymbolScheme and URL
serializer/resolver but at least they could serve for other needs).
Actually we do have a parser for this since this syntax already exist
for String to EntityReference automatic conversion in Velocity
scripts. See
https://github.com/xwiki/xwiki-platform/blob/master/xwiki-platform-core/xwi….
You could move this in a EnityReferenceResolver and use the resolver
in the converter if you prefer.
Didn’t know about this one :) Now we still need to write a resolver (and serializer) and
do it right. It might be better to implement it as a ResourceRefefenceResolver/Serializer
if we want to be more generic though.
I’ve also been wanting to refactor the link resource reference parser as a Resource module
resolver.
You can't really decide later to add a prefix
without breaking the URL
scheme. When you already have a prefix you can add other prefixes
without breaking anything.
Yes and I don’t think we care that much about breaking the URL scheme since it’s internal.
Breaking the API to generate the URL is a bit more problematic but still acceptable if we
keep it internal for now.
Anyway let’s try to do it right. I’m just unsure that I’ll have the time to work on this
any time soon but at least we now have this thread to know what we want when someone will
next have the time to work on this.
The right API being already EntityReference
based it's just about converting the String to an EntityReference
instead of a DocumentReference.
The path is already module dependent so no need
to add a module
related element in the URL scheme IMO. We can indicate that a good
practice for custom modules resources is to start the path with some
module unique identifier but it does not really need to have any
special meaning in the URL itself.
I don’t agree. It’s the same as typed api vs untyped API. If you don’t put the module id
in the API then it’s up to best practices and we do know that best practices are hard to
enforce. So I’m strongly in favor of having it in the API (i.e. in the URL).
As long as the choice is on module side it's never really fully safe
but sure it's a bit safer. My point was more that for some core stuff
we may not need any module id, producing simpler URL, but sure we can
always find some module id that make sense.
Exactly like module that manipulate
permanent and temporary directories don't get a dedicated folder, they
just decide to put their stuff in some mymodule/ subforlder (or not).
That’s not right IMO. The temporary API should return an isolated location for a given
module so that it has no risk of clashing with another module. It shouldn’t be the goal of
a given module to ensure this. It should be a service of the temporary API. We could have
2 APIs but the main one that modules should use should be the one returning an isolated
and safe location.
According to this logic it means we need to refactor the Environment
API accordingly. Same thing for configurations and translations in
which property names are up to each module which usually put itself as
property name prefix but could forget, etc.
Yes potentially. As we can see in the translations, it’s hard to have a single naming
convention that is respected.
Thanks
-Vincent
Thanks
> -Vincent
>
>> Thanks
>>> -Vincent
>>>
>>>> BTW this means that my URL entity reference serializer/resolver are no
useful for this ;) (they’d still be useful for the “reference” url scheme though). We
could simply take a String an replace the “/“ and “\” with some other symbols and do the
same when parsing the URL..
>>>>
>>>> So the generic format would be:
>>>>
>>>> http://<server>/<context>/tmp/<module
id>/<serialized reference/id representing the resource>/<module-dependent
resource path>
>>>>
>>>> The <serialized reference/id representing the resource> wouldn’t be
able to be empty though since <module-dependent resource path> is non-fixed length
(e.g. “a/b/c”).
>>>>
>>>> WDYT?
>>>>
>>>> Thanks
>>>> -Vincent
>>>>
>>>>> Now one job of the tmp resource is also to check access right so we
>>>>> need to pass it an entity reference on which to test the right when
a
>>>>> right check is required. The alternative being to end up with the
>>>>> reference both in the path (to avoir collisions) and as some URL
>>>>> parameter which is not nice I guess what you propose it ok as long
as
>>>>> empty reference is supported (i.e. don't test the right and just
go
>>>>> return the file associated to the path) as in
>>>>>
http://mydomain/xwiki/tmp/mymodule//I/don't/care/about/right.png
>>>>>
>>>>> Making the tmp resource generic enough to be just an entry point for
>>>>> calling some module which then do whatever it wants would just be a
>>>>> duplicate of resource handler framework but maybe we just don't
really
>>>>> need this anymore central temp resource entry point since now that
we
>>>>> have a generic resource handler framework ?
>>>>>
>>>>> On Fri, Apr 15, 2016 at 9:26 AM, Vincent Massol
<vincent(a)massol.net> wrote:
>>>>>> @Thomas: are you ok with the proposed format:
>>>>>>
>>>>>> http://<server>/<context>/tmp/<module
id>/<serialized owner document reference>/<module-dependent resource path>
>>>>>>
>>>>>> ?
>>>>>>
>>>>>> Thanks
>>>>>> -Vincent
>>>>>>
>>>>>>> On 14 Apr 2016, at 17:55, Thomas Mortagne
<thomas.mortagne(a)xwiki.com> wrote:
>>>>>>>
>>>>>>> On Thu, Apr 14, 2016 at 4:52 PM, Marius Dumitru Florea
>>>>>>> <mariusdumitru.florea(a)xwiki.com> wrote:
>>>>>>>> On Thu, Apr 14, 2016 at 5:43 PM, Vincent Massol
<vincent(a)massol.net> wrote:
>>>>>>>>
>>>>>>>>> Hi devs,
>>>>>>>>>
>>>>>>>>> I’m implementing
http://jira.xwiki.org/browse/XWIKI-10375 ("Refactor the
>>>>>>>>> temporary resource concept inside the Resource
module”) and I need to
>>>>>>>>> define a URL format for the new “tmp” resource type.
>>>>>>>>>
>>>>>>>>> I’m proposing the following:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>> http://<server>/<context>/tmp/<module
id>/<serialized owner document
>>>>>>>>> reference>/<module-dependent resource path>
>>>>>>>>>
>>>>>>>>
>>>>>>>> Serialized document reference uses backslash to escape
special characters
>>>>>>>> which breaks the URL in Tomcat for security reasons.
>>>>>>>
>>>>>>> Badly configured Tomcat does not like slash but are you sure
about backslash ?
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> This is based on the existing
TemporaryResourceReference at:
>>>>>>>>>
>>>>>>>>>
https://github.com/xwiki/xwiki-platform/blob/96caad053c14fc5546e9bc141bc284…
>>>>>>>>>
>>>>>>>>> For example:
>>>>>>>>>
>>>>>>>>> http://
>>>>>>>>>
<server>/<context>/tmp/officeviewer/A.B.WebHome/Q29tcGFueSBQcmVzZW50YXRpb24ucHB0/Company+Presentation-slide0.jpg
>>>>>>>>>
>>>>>>>>> Note that in this example from the officeviewer macro
the module-dependent
>>>>>>>>> resource path consists in:
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> - base64(name of office attachment +
hashcode(parameters))
>>>>>>>>>
>>>>>>>>
>>>>>>>> See
http://jira.xwiki.org/browse/XWIKI-11528 for the
rationale behind it. I
>>>>>>>> was trying to avoid backslash (from the serialized
attachment reference) in
>>>>>>>> the URL.
>>>>>>>>
>>>>>>>>
>>>>>>>>> - generated image name from PPT
>>>>>>>>>
>>>>>>>>> In this case, the implementation would generate the
following file:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
[TMPDIR]/officeviewer/A/B/WebHome/Q29tcGFueSBQcmVzZW50YXRpb24ucHB0/Company+Presentation-slide0.jpg
>>>>>>>>>
>>>>>>>>> WDYT?
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>> -Vincent