logic to prevent overwriting release-artifacts

[Marc Lustig]

Hi,

I have one more feature request, which I consider cruical to improve the consistency of the repository-logic. Before filing another jira feature-request, I'd like to get some feedback.

For snapshots, it's in the nature of the process to overwrite existing versions. By implementing an afterSnapshotDeployment event (in one or another way) one can ensure that snapshots are not added with a new timestamp, but replacing the existing one - independent of the configuration of the maven-deploy-plugin in the project's pom-file.

For releases however, AFAIK, Archiva offers no mechanism to prevent overwriting existing release-versions. In my and all of my collegues opinion, this heavily breaks the contract.
A release-artifact is supposed to be immutable. It's not just the concept. It has also practical implications for the developers. People download version x from Archiva, some guy wants to do a quick-fix and deploys the artifact as the same version. As a consequence, different developers will have difference editions of this artifact-version in their local repo. What a mess!
We had this case a couple of times and people were right to complain that Archiva has not prohibited overwriting the artifact.

So, I propose the following logic to get implemented in Archiva.
First of all, by default a release-artifact that is already published in Archiva may NOT be overwritten.

Furthermore, to provide flexibility for the admins, a new permission property "user may overwrite existing artifacts" should be introduced. Only users that have this permission may overwrite existing artifacts.

I think this is a pretty consistent solution.
Not quite sure because I don't know the code, but the effort for the implementation seems rather small to me, compared with the benefit.

[brettporter]

On 24/09/2009, at 10:29 PM, Marc Lustig wrote:

> So, I propose the following logic to get implemented in Archiva.
> First of all, by default a release-artifact that is already  
> published in
> Archiva may NOT be overwritten.

+1

>
> Furthermore, to provide flexibility for the admins, a new permission
> property "user may overwrite existing artifacts" should be  
> introduced. Only
> users that have this permission may overwrite existing artifacts.

That may not even be necessary with the existence of delete artifact -  
they can log in and remove it to be allowed to deploy again.

>
> I think this is a pretty consistent solution.
> Not quite sure because I don't know the code, but the effort for the
> implementation seems rather small to me, compared with the benefit.

Wholeheartedly agree! I believe there is already an issue for this if  
you'd like to work on it.

Cheers,
Brett

[Marc Lustig]

OK, I agree that instead of adding a permission for overwriting artifacts it should be sufficient to delete that particlar artifact. The case when you have to overwrite a whole bunch of artifacts in once should be rather rare.

Yeah, I already filed that a while ago as MRM-992.
And even that was a duplicate for 747.
OK, I will see if I find the time to fix it. The first barrier is to get acqainted with the code...

brettporter wrote:

On 24/09/2009, at 10:29 PM, Marc Lustig wrote:

> So, I propose the following logic to get implemented in Archiva.
> First of all, by default a release-artifact that is already  
> published in
> Archiva may NOT be overwritten.

+1

>
> Furthermore, to provide flexibility for the admins, a new permission
> property "user may overwrite existing artifacts" should be  
> introduced. Only
> users that have this permission may overwrite existing artifacts.

That may not even be necessary with the existence of delete artifact -  
they can log in and remove it to be allowed to deploy again.

>
> I think this is a pretty consistent solution.
> Not quite sure because I don't know the code, but the effort for the
> implementation seems rather small to me, compared with the benefit.

Wholeheartedly agree! I believe there is already an issue for this if  
you'd like to work on it.

Cheers,
Brett

[brettporter]

On 25/09/2009, at 7:56 PM, Marc Lustig wrote:

>
> OK, I agree that instead of adding a permission for overwriting  
> artifacts it
> should be sufficient to delete that particlar artifact. The case  
> when you
> have to overwrite a whole bunch of artifacts in once should be  
> rather rare.
>
> Yeah, I already filed that a while ago as MRM-992.
> And even that was a duplicate for 747.
> OK, I will see if I find the time to fix it. The first barrier is to  
> get
> acqainted with the code...

Let us know how we can help!

- Brett

[Marc Lustig]

yes please - the initial call to deploy the artifact is done in #124 in RepositoryServlet right?
(Deng mentioned that the repo-consumer is called immediately after deploying the artifact, but I guess the check should be done before the deployment is triggered.)

So could you give me a hint how to retrieve the information necessary to do the check:
- target repo and location in the repo in the fs (?)
- artifact data (groupId, artifactId, ...)

What is the proper way to do that lookup? via File.exists(), or not better using some Archiva-method like
SomeStaticSingleton. getRepo(reponame).artfifactExists(groupId, artifactId, ....)

That would be better software design than looking up in the fs...

brettporter wrote:

On 25/09/2009, at 7:56 PM, Marc Lustig wrote:

>
> OK, I agree that instead of adding a permission for overwriting  
> artifacts it
> should be sufficient to delete that particlar artifact. The case  
> when you
> have to overwrite a whole bunch of artifacts in once should be  
> rather rare.
>
> Yeah, I already filed that a while ago as MRM-992.
> And even that was a duplicate for 747.
> OK, I will see if I find the time to fix it. The first barrier is to  
> get
> acqainted with the code...

Let us know how we can help!

- Brett

[Marc Lustig]

Deng, Brett, et al

if you give me some instructions to implement this straight away, I will be glad to do that.



yes please - the initial call to deploy the artifact is done in #124 in RepositoryServlet right?
(Deng mentioned that the repo-consumer is called immediately after deploying the artifact, but I guess the check should be done before the deployment is triggered.)

So could you give me a hint how to retrieve the information necessary to do the check:
- target repo and location in the repo in the fs (?)
- artifact data (groupId, artifactId, ...)

What is the proper way to do that lookup? via File.exists(), or not better using some Archiva-method like
SomeStaticSingleton. getRepo(reponame).artfifactExists(groupId, artifactId, ....)

That would be better software design than looking up in the fs...

brettporter wrote:

On 25/09/2009, at 7:56 PM, Marc Lustig wrote:

>
> OK, I agree that instead of adding a permission for overwriting  
> artifacts it
> should be sufficient to delete that particlar artifact. The case  
> when you
> have to overwrite a whole bunch of artifacts in once should be  
> rather rare.
>
> Yeah, I already filed that a while ago as MRM-992.
> And even that was a duplicate for 747.
> OK, I will see if I find the time to fix it. The first barrier is to  
> get
> acqainted with the code...

Let us know how we can help!

- Brett

[Deng Ching-2]

Hi Marc,

Please see in-line comments below :)

On Mon, Oct 5, 2009 at 7:32 PM, Marc Lustig <ml@...> wrote:

>
> Deng, Brett, et al
>
> if you give me some instructions to implement this straight away, I will be
> glad to do that.
>
>
>
> Marc Lustig wrote:
> >
> > yes please - the initial call to deploy the artifact is done in #124 in
> > RepositoryServlet right?
> > (Deng mentioned that the repo-consumer is called immediately after
> > deploying the artifact, but I guess the check should be done before the
> > deployment is triggered.)
> >
> > So could you give me a hint how to retrieve the information necessary to
> > do the check:
> > - target repo and location in the repo in the fs (?)
>

You can get this from ArchivaConfiguration, I think this is already a
component of the repo-consumer.


> > - artifact data (groupId, artifactId, ...)
>

This is usually passed in to the repo consumer. If you want to check for the
existing artifact in the database, you can get it through the ArtifactDAO..


> >
> > What is the proper way to do that lookup? via File.exists(), or not
> better
> > using some Archiva-method like
> > SomeStaticSingleton. getRepo(reponame).artfifactExists(groupId,
> > artifactId, ....)
> >
>

I think there is a component for this in the repository-layer module, I'll
get back to you on this one :)

Thanks,
Deng

> > That would be better software design than looking up in the fs...
> >
> >
> >
> >
> >
> >
> > brettporter wrote:
> >>
> >>
> >> On 25/09/2009, at 7:56 PM, Marc Lustig wrote:
> >>
> >>>
> >>> OK, I agree that instead of adding a permission for overwriting
> >>> artifacts it
> >>> should be sufficient to delete that particlar artifact. The case
> >>> when you
> >>> have to overwrite a whole bunch of artifacts in once should be
> >>> rather rare.
> >>>
> >>> Yeah, I already filed that a while ago as MRM-992.
> >>> And even that was a duplicate for 747.
> >>> OK, I will see if I find the time to fix it. The first barrier is to
> >>> get
> >>> acqainted with the code...
> >>
> >> Let us know how we can help!
> >>
> >> - Brett
> >>
> >>
> >>
> >
> >
>
> --
> View this message in context:
> http://www.nabble.com/logic-to-prevent-overwriting-release-artifacts-tp25564416p25749141.html
> Sent from the archiva-dev mailing list archive at Nabble.com.
>
>

[Marc Lustig]

Ok, thanks, so if I understand you correct, we should not the RepositoryServlet, but the repo-consumer that you proposed, right?

Deng Ching-2 wrote:

Hi Marc,

Please see in-line comments below :)

On Mon, Oct 5, 2009 at 7:32 PM, Marc Lustig <[e-mail]> wrote:

>
> Deng, Brett, et al
>
> if you give me some instructions to implement this straight away, I will be
> glad to do that.
>
>
>
> Marc Lustig wrote:
> >
> > yes please - the initial call to deploy the artifact is done in #124 in
> > RepositoryServlet right?
> > (Deng mentioned that the repo-consumer is called immediately after
> > deploying the artifact, but I guess the check should be done before the
> > deployment is triggered.)
> >
> > So could you give me a hint how to retrieve the information necessary to
> > do the check:
> > - target repo and location in the repo in the fs (?)
>

You can get this from ArchivaConfiguration, I think this is already a
component of the repo-consumer.


> > - artifact data (groupId, artifactId, ...)
>

This is usually passed in to the repo consumer. If you want to check for the
existing artifact in the database, you can get it through the ArtifactDAO..


> >
> > What is the proper way to do that lookup? via File.exists(), or not
> better
> > using some Archiva-method like
> > SomeStaticSingleton. getRepo(reponame).artfifactExists(groupId,
> > artifactId, ....)
> >
>

I think there is a component for this in the repository-layer module, I'll
get back to you on this one :)

Thanks,
Deng


> > That would be better software design than looking up in the fs...
> >
> >
> >
> >
> >
> >
> > brettporter wrote:
> >>
> >>
> >> On 25/09/2009, at 7:56 PM, Marc Lustig wrote:
> >>
> >>>
> >>> OK, I agree that instead of adding a permission for overwriting
> >>> artifacts it
> >>> should be sufficient to delete that particlar artifact. The case
> >>> when you
> >>> have to overwrite a whole bunch of artifacts in once should be
> >>> rather rare.
> >>>
> >>> Yeah, I already filed that a while ago as MRM-992.
> >>> And even that was a duplicate for 747.
> >>> OK, I will see if I find the time to fix it. The first barrier is to
> >>> get
> >>> acqainted with the code...
> >>
> >> Let us know how we can help!
> >>
> >> - Brett
> >>
> >>
> >>
> >
> >
>
> --
> View this message in context:
> http://www.nabble.com/logic-to-prevent-overwriting-release-artifacts-tp25564416p25749141.html
> Sent from the archiva-dev mailing list archive at Nabble.com.
>
>

[Deng Ching-2]

Hi Marc,

Sorry for the confusion, I think the blocking of overwriting released
artifacts should be placed in the ArchivaDavResourceFactory instead of the
RepositoryServlet or RepositoryContentConsumer since that handles the
resource requests and that's where the artifact data is determined. You
might want to take a look at the processRepository(..) method where the PUT
requests are handled.

There's also the upload from the web UI to consider for this feature as it
is entirely separate from the DAV deployment. This is in turn handled by the
UploadAction in archiva-webapp :)

Thanks,
Deng

On Mon, Oct 5, 2009 at 11:58 PM, Marc Lustig <ml@...> wrote:

>
> Ok, thanks, so if I understand you correct, we should not the
> RepositoryServlet, but the repo-consumer that you proposed, right?
>
>
> Deng Ching-2 wrote:
> >
> > Hi Marc,
> >
> > Please see in-line comments below :)
> >
> > On Mon, Oct 5, 2009 at 7:32 PM, Marc Lustig <ml@...> wrote:
> >
> >>
> >> Deng, Brett, et al
> >>
> >> if you give me some instructions to implement this straight away, I will
> >> be
> >> glad to do that.
> >>
> >>
> >>
> >> Marc Lustig wrote:
> >> >
> >> > yes please - the initial call to deploy the artifact is done in #124
> in
> >> > RepositoryServlet right?
> >> > (Deng mentioned that the repo-consumer is called immediately after
> >> > deploying the artifact, but I guess the check should be done before
> the
> >> > deployment is triggered.)
> >> >
> >> > So could you give me a hint how to retrieve the information necessary
> >> to
> >> > do the check:
> >> > - target repo and location in the repo in the fs (?)
> >>
> >
> > You can get this from ArchivaConfiguration, I think this is already a
> > component of the repo-consumer.
> >
> >
> >> > - artifact data (groupId, artifactId, ...)
> >>
> >
> > This is usually passed in to the repo consumer. If you want to check for
> > the
> > existing artifact in the database, you can get it through the
> > ArtifactDAO..
> >
> >
> >> >
> >> > What is the proper way to do that lookup? via File.exists(), or not
> >> better
> >> > using some Archiva-method like
> >> > SomeStaticSingleton. getRepo(reponame).artfifactExists(groupId,
> >> > artifactId, ....)
> >> >
> >>
> >
> > I think there is a component for this in the repository-layer module,
> I'll
> > get back to you on this one :)
> >
> > Thanks,
> > Deng
> >
> >
> >> > That would be better software design than looking up in the fs...
> >> >
> >> >
> >> >
> >> >
> >> >
> >> >
> >> > brettporter wrote:
> >> >>
> >> >>
> >> >> On 25/09/2009, at 7:56 PM, Marc Lustig wrote:
> >> >>
> >> >>>
> >> >>> OK, I agree that instead of adding a permission for overwriting
> >> >>> artifacts it
> >> >>> should be sufficient to delete that particlar artifact. The case
> >> >>> when you
> >> >>> have to overwrite a whole bunch of artifacts in once should be
> >> >>> rather rare.
> >> >>>
> >> >>> Yeah, I already filed that a while ago as MRM-992.
> >> >>> And even that was a duplicate for 747.
> >> >>> OK, I will see if I find the time to fix it. The first barrier is to
> >> >>> get
> >> >>> acqainted with the code...
> >> >>
> >> >> Let us know how we can help!
> >> >>
> >> >> - Brett
> >> >>
> >> >>
> >> >>
> >> >
> >> >
> >>
> >> --
> >> View this message in context:
> >>
> http://www.nabble.com/logic-to-prevent-overwriting-release-artifacts-tp25564416p25749141.html
> >> Sent from the archiva-dev mailing list archive at Nabble.com.
> >>
> >>
> >
> >
>
> --
> View this message in context:
> http://www.nabble.com/logic-to-prevent-overwriting-release-artifacts-tp25564416p25753420.html
> Sent from the archiva-dev mailing list archive at Nabble.com.
>
>

[Marc Lustig]

Hi all,

please find the patch here: http://jira.codehaus.org/browse/MRM-747

For us it would be important to get this integrated into the trunk asap, so that 1.2.3 will already contain it.
Can I please ask to review the patch and let me know if something should be changed.

thanks
Marc

Deng Ching-2 wrote:

Hi Marc,

Sorry for the confusion, I think the blocking of overwriting released
artifacts should be placed in the ArchivaDavResourceFactory instead of the
RepositoryServlet or RepositoryContentConsumer since that handles the
resource requests and that's where the artifact data is determined. You
might want to take a look at the processRepository(..) method where the PUT
requests are handled.

There's also the upload from the web UI to consider for this feature as it
is entirely separate from the DAV deployment. This is in turn handled by the
UploadAction in archiva-webapp :)

Thanks,
Deng

On Mon, Oct 5, 2009 at 11:58 PM, Marc Lustig <[e-mail]> wrote:

>
> Ok, thanks, so if I understand you correct, we should not the
> RepositoryServlet, but the repo-consumer that you proposed, right?
>
>
> Deng Ching-2 wrote:
> >
> > Hi Marc,
> >
> > Please see in-line comments below :)
> >
> > On Mon, Oct 5, 2009 at 7:32 PM, Marc Lustig <[e-mail]> wrote:
> >
> >>
> >> Deng, Brett, et al
> >>
> >> if you give me some instructions to implement this straight away, I will
> >> be
> >> glad to do that.
> >>
> >>
> >>
> >> Marc Lustig wrote:
> >> >
> >> > yes please - the initial call to deploy the artifact is done in #124
> in
> >> > RepositoryServlet right?
> >> > (Deng mentioned that the repo-consumer is called immediately after
> >> > deploying the artifact, but I guess the check should be done before
> the
> >> > deployment is triggered.)
> >> >
> >> > So could you give me a hint how to retrieve the information necessary
> >> to
> >> > do the check:
> >> > - target repo and location in the repo in the fs (?)
> >>
> >
> > You can get this from ArchivaConfiguration, I think this is already a
> > component of the repo-consumer.
> >
> >
> >> > - artifact data (groupId, artifactId, ...)
> >>
> >
> > This is usually passed in to the repo consumer. If you want to check for
> > the
> > existing artifact in the database, you can get it through the
> > ArtifactDAO..
> >
> >
> >> >
> >> > What is the proper way to do that lookup? via File.exists(), or not
> >> better
> >> > using some Archiva-method like
> >> > SomeStaticSingleton. getRepo(reponame).artfifactExists(groupId,
> >> > artifactId, ....)
> >> >
> >>
> >
> > I think there is a component for this in the repository-layer module,
> I'll
> > get back to you on this one :)
> >
> > Thanks,
> > Deng
> >
> >
> >> > That would be better software design than looking up in the fs...
> >> >
> >> >
> >> >
> >> >
> >> >
> >> >
> >> > brettporter wrote:
> >> >>
> >> >>
> >> >> On 25/09/2009, at 7:56 PM, Marc Lustig wrote:
> >> >>
> >> >>>
> >> >>> OK, I agree that instead of adding a permission for overwriting
> >> >>> artifacts it
> >> >>> should be sufficient to delete that particlar artifact. The case
> >> >>> when you
> >> >>> have to overwrite a whole bunch of artifacts in once should be
> >> >>> rather rare.
> >> >>>
> >> >>> Yeah, I already filed that a while ago as MRM-992.
> >> >>> And even that was a duplicate for 747.
> >> >>> OK, I will see if I find the time to fix it. The first barrier is to
> >> >>> get
> >> >>> acqainted with the code...
> >> >>
> >> >> Let us know how we can help!
> >> >>
> >> >> - Brett
> >> >>
> >> >>
> >> >>
> >> >
> >> >
> >>
> >> --
> >> View this message in context:
> >>
> http://www.nabble.com/logic-to-prevent-overwriting-release-artifacts-tp25564416p25749141.html
> >> Sent from the archiva-dev mailing list archive at Nabble.com.
> >>
> >>
> >
> >
>
> --
> View this message in context:
> http://www.nabble.com/logic-to-prevent-overwriting-release-artifacts-tp25564416p25753420.html
> Sent from the archiva-dev mailing list archive at Nabble.com.
>
>

[Marc Lustig]

To implement the unit-test it would be quite helpful to execute the unit-test based on a runtime-environment. Does the Archivas build-process allow to execute unit-tests in a runtime-environment?
Otherwise you will have to mock all kinds of objects, which doesn't reflect the real scenario in the end and also is a lot of work.

[Deng Ching-2]

Hi Marc,

I think you can pattern and add your test to the ones for the
RepositoryServlet where all the webdav tests are. They're sort of an
integration test for the webdav components.

Thanks,
Deng

On Fri, Oct 9, 2009 at 2:59 PM, Marc Lustig <ml@...> wrote:

>
> To implement the unit-test it would be quite helpful to execute the
> unit-test
> based on a runtime-environment. Does the Archivas build-process allow to
> execute unit-tests in a runtime-environment?
> Otherwise you will have to mock all kinds of objects, which doesn't reflect
> the real scenario in the end and also is a lot of work.
> --
> View this message in context:
> http://www.nabble.com/logic-to-prevent-overwriting-release-artifacts-tp25564416p25816355.html
> Sent from the archiva-dev mailing list archive at Nabble.com.
>
>