You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Rolf Kulemann <ro...@apache.org> on 2004/04/05 13:19:47 UTC

[PROP] Repository interface

[Related to http://issues.apache.org/bugzilla/show_bug.cgi?id=28189]

<quote src="Guido Capser">
As for WebDAVRepsoitoryVersioningHelper.setVersioned(): I don't think it
is a good idea to throw an 
UnsupportedOperationException only under certain conditions.
</quote>

Agreed.

Maybe it makes sense to split setVersioned(String, boolean) into two
methods

1.) setVersioned(String)
2.) unsetVersioned(String)

in order to be more flexible in the implementation decisions?

<quote src="Guido Capser">
A setVersioned(uri, false) might be implemented by deleting and  
recreating the 
resource (making copies for safety and checking for auto-versioning).
</quote>

Ehem, how can I check for auto-versioning?

-- 
Regards,

    Rolf Kulemann


Re: [PROP] Repository interface

Posted by Guido Casper <gc...@s-und-n.de>.
Rolf Kulemann wrote:
>>>Maybe we should simply return false on setVersioned(String, false),
>>>because there are to many cases where we get inconsistent data or have
>>>other problems during moving with coliding file names etc.
>>
>>Did just that for the time being.
>>
>>However I think this might be properly implemented by:
>>-copy to a temporary resource
>>-check wether the temporary resource is version-controlled
>>-move the temporary resource back overriding the original resource
>>
> 
> 
> We also need to set appropriate locks if supported, imho.

Yes.

Guido

-- 
Guido Casper
-------------------------------------------------
S&N AG, Competence Center Open Source
                     Tel.: +49-5251-1581-87
Klingenderstr. 5    mailto:gcasper@s-und-n.de
D-33100 Paderborn   http://www.s-und-n.de
-------------------------------------------------

Re: [PROP] Repository interface

Posted by Rolf Kulemann <ro...@apache.org>.
On Mon, 2004-04-05 at 16:33, Guido Casper wrote:
> Rolf Kulemann wrote:
> 
> > On Mon, 2004-04-05 at 13:38, Guido Casper wrote:
> > 
> >>Rolf Kulemann wrote:
> >>I just changed the code a little bit. However it's now a little hacky 
> >>and needs completion as after the copy you should check wether the 
> >>destination is version controlled and if that's the case undo the move 
> >>(to keep the old version history) and return false.
> >>
> >>I don't know the details of how to check wether a resource is under 
> >>version control right now (would have to check the DeltaV spec).
> >>
> > 
> > 
> > Maybe we should simply return false on setVersioned(String, false),
> > because there are to many cases where we get inconsistent data or have
> > other problems during moving with coliding file names etc.
> 
> Did just that for the time being.
> 
> However I think this might be properly implemented by:
> -copy to a temporary resource
> -check wether the temporary resource is version-controlled
> -move the temporary resource back overriding the original resource
> 

We also need to set appropriate locks if supported, imho.

-- 
Regards,

    Rolf Kulemann


Re: [PROP] Repository interface

Posted by Guido Casper <gc...@s-und-n.de>.
Rolf Kulemann wrote:

> On Mon, 2004-04-05 at 13:38, Guido Casper wrote:
> 
>>Rolf Kulemann wrote:
>>I just changed the code a little bit. However it's now a little hacky 
>>and needs completion as after the copy you should check wether the 
>>destination is version controlled and if that's the case undo the move 
>>(to keep the old version history) and return false.
>>
>>I don't know the details of how to check wether a resource is under 
>>version control right now (would have to check the DeltaV spec).
>>
> 
> 
> Maybe we should simply return false on setVersioned(String, false),
> because there are to many cases where we get inconsistent data or have
> other problems during moving with coliding file names etc.

Did just that for the time being.

However I think this might be properly implemented by:
-copy to a temporary resource
-check wether the temporary resource is version-controlled
-move the temporary resource back overriding the original resource

The open question is what happen to the version history. I tend to 
having a global configuration whether to keep histories of deleted 
resources or remove them (as keeping histories of deleted resources 
might bloat the repository a lot).

Guido

-- 
Guido Casper
-------------------------------------------------
S&N AG, Competence Center Open Source
                     Tel.: +49-5251-1581-87
Klingenderstr. 5    mailto:gcasper@s-und-n.de
D-33100 Paderborn   http://www.s-und-n.de
-------------------------------------------------

Re: [PROP] Repository interface

Posted by Rolf Kulemann <ro...@apache.org>.
On Mon, 2004-04-05 at 13:38, Guido Casper wrote:
> Rolf Kulemann wrote:
> I just changed the code a little bit. However it's now a little hacky 
> and needs completion as after the copy you should check wether the 
> destination is version controlled and if that's the case undo the move 
> (to keep the old version history) and return false.
> 
> I don't know the details of how to check wether a resource is under 
> version control right now (would have to check the DeltaV spec).
> 

Maybe we should simply return false on setVersioned(String, false),
because there are to many cases where we get inconsistent data or have
other problems during moving with coliding file names etc.

Makes sense?

-- 
Regards,

    Rolf Kulemann


Re: [PROP] Repository interface

Posted by Guido Casper <gc...@s-und-n.de>.
Rolf Kulemann wrote:
> On Mon, 2004-04-05 at 13:38, Guido Casper wrote:
> 
>>Rolf Kulemann wrote:
>>
>>>[Related to http://issues.apache.org/bugzilla/show_bug.cgi?id=28189]
>>>
>>><quote src="Guido Capser">
>>>As for WebDAVRepsoitoryVersioningHelper.setVersioned(): I don't think it
>>>is a good idea to throw an 
>>>UnsupportedOperationException only under certain conditions.
>>></quote>
>>>
>>>Agreed.
>>>
>>>Maybe it makes sense to split setVersioned(String, boolean) into two
>>>methods
>>>
>>>1.) setVersioned(String)
>>>2.) unsetVersioned(String)
>>>
>>>in order to be more flexible in the implementation decisions?
>>
>>Why should that be more flexible?
> 
> 
> Because I can decide if I implement both or only one method. The reason
> for the split is, I can use the NotSupportedException more accurate.

Well, throwing UnsupportedOperationException is not my final goal for 
either :-)

I guess having too many of those might indicate an unappropriate interface.

> 
> 
>>TBH I'm not so sure wether these should be part of the Repository 
>>interface at all as it looks like being geared towards WebDAV only (but 
>>I don't know right now).
> 
> 
> Ok, I didn't mean the Repsoitory interface, was a litlle bit confusing
> maybe, but the RepositoryVersioningHelper. And if a repository does not
> support versioning it has no helper at all.

Yes, but I was talking about the RepositoryVersioningHelper as well (a 
single repository having both versioned and non-versioned resources) 
which is exactly the point of my comment above.

I think it might be a wise move to first gather experiences with other 
implementations (instead of adding more operations coming directly from 
a WebDAV context).

Guido

-- 
Guido Casper
-------------------------------------------------
S&N AG, Competence Center Open Source
                     Tel.: +49-5251-1581-87
Klingenderstr. 5    mailto:gcasper@s-und-n.de
D-33100 Paderborn   http://www.s-und-n.de
-------------------------------------------------

Re: [PROP] Repository interface

Posted by Rolf Kulemann <ro...@apache.org>.
On Mon, 2004-04-05 at 13:38, Guido Casper wrote:
> Rolf Kulemann wrote:
> > [Related to http://issues.apache.org/bugzilla/show_bug.cgi?id=28189]
> > 
> > <quote src="Guido Capser">
> > As for WebDAVRepsoitoryVersioningHelper.setVersioned(): I don't think it
> > is a good idea to throw an 
> > UnsupportedOperationException only under certain conditions.
> > </quote>
> > 
> > Agreed.
> > 
> > Maybe it makes sense to split setVersioned(String, boolean) into two
> > methods
> > 
> > 1.) setVersioned(String)
> > 2.) unsetVersioned(String)
> > 
> > in order to be more flexible in the implementation decisions?
> 
> Why should that be more flexible?

Because I can decide if I implement both or only one method. The reason
for the split is, I can use the NotSupportedException more accurate.

> 
> TBH I'm not so sure wether these should be part of the Repository 
> interface at all as it looks like being geared towards WebDAV only (but 
> I don't know right now).

Ok, I didn't mean the Repsoitory interface, was a litlle bit confusing
maybe, but the RepositoryVersioningHelper. And if a repository does not
support versioning it has no helper at all.

> 
> > 
> > <quote src="Guido Capser">
> > A setVersioned(uri, false) might be implemented by deleting and  
> > recreating the 
> > resource (making copies for safety and checking for auto-versioning).
> > </quote>
> > 
> > Ehem, how can I check for auto-versioning?
> 
> It's a property of the resource. Wether this property is already set 
> upon resource creation usually depends on some server setting.
> 
> I just changed the code a little bit. However it's now a little hacky 
> and needs completion as after the copy you should check wether the 
> destination is version controlled and if that's the case undo the move 
> (to keep the old version history) and return false.
> 
> I don't know the details of how to check wether a resource is under 
> version control right now (would have to check the DeltaV spec).

I will do that. Thanks.
-- 
Regards,

    Rolf Kulemann


Re: [PROP] Repository interface

Posted by Guido Casper <gc...@s-und-n.de>.
Rolf Kulemann wrote:
> [Related to http://issues.apache.org/bugzilla/show_bug.cgi?id=28189]
> 
> <quote src="Guido Capser">
> As for WebDAVRepsoitoryVersioningHelper.setVersioned(): I don't think it
> is a good idea to throw an 
> UnsupportedOperationException only under certain conditions.
> </quote>
> 
> Agreed.
> 
> Maybe it makes sense to split setVersioned(String, boolean) into two
> methods
> 
> 1.) setVersioned(String)
> 2.) unsetVersioned(String)
> 
> in order to be more flexible in the implementation decisions?

Why should that be more flexible?

TBH I'm not so sure wether these should be part of the Repository 
interface at all as it looks like being geared towards WebDAV only (but 
I don't know right now).

> 
> <quote src="Guido Capser">
> A setVersioned(uri, false) might be implemented by deleting and  
> recreating the 
> resource (making copies for safety and checking for auto-versioning).
> </quote>
> 
> Ehem, how can I check for auto-versioning?

It's a property of the resource. Wether this property is already set 
upon resource creation usually depends on some server setting.

I just changed the code a little bit. However it's now a little hacky 
and needs completion as after the copy you should check wether the 
destination is version controlled and if that's the case undo the move 
(to keep the old version history) and return false.

I don't know the details of how to check wether a resource is under 
version control right now (would have to check the DeltaV spec).

Guido

-- 
Guido Casper
-------------------------------------------------
S&N AG, Competence Center Open Source
                     Tel.: +49-5251-1581-87
Klingenderstr. 5    mailto:gcasper@s-und-n.de
D-33100 Paderborn   http://www.s-und-n.de
-------------------------------------------------