Mailing-List: contact dev-help@archiva.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@archiva.apache.org
Received-SPF: pass (athena.apache.org: domain of odeaching@gmail.com
 designates 209.85.160.60 as permitted sender)
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=mime-version:sender:in-reply-to:references:date
         :x-google-sender-auth:message-id:subject:from:to:content-type;
        b=cpMbnYW0elcw0oP0EFrTTsRHKj9nIIMR8orm7sbXT/eBcswG3YyMJ5dpWec273u2V8
         1vhXRmIlmC125ZffLWpHAh0zcOll0/ufihkrsF1Rwqz8S+kUqu8+8eArIwQYdVW6gtbZ
         VDJkQ3LcfaSmAtoQmN81OHlhBcxGo4x6MIdCQ=
MIME-Version: 1.0
Sender: odeaching@gmail.com
In-Reply-To: <9EDE9FFD-8D14-4C79-829C-2B7E9C1F935A@apache.org>
References: <20091013103626.6943723888FF@eris.apache.org>
	 <5D409FF3-84DF-4F8E-BA47-781D179D0E62@apache.org>
	 <8667b1bd0910140103m4a127c26g3f29d739f8c31458@mail.gmail.com>
	 <9EDE9FFD-8D14-4C79-829C-2B7E9C1F935A@apache.org>
Date: Thu, 15 Oct 2009 15:08:41 +0800
Message-ID: <8667b1bd0910150008x110dc2a7q782785d2b5b9a842@mail.gmail.com>
Subject: Re: svn commit: r824677 - in
 /archiva/trunk/archiva-modules/archiva-web/archiva-webdav/src:
	main/java/org/apache/maven/archiva/webdav/
 test/java/org/apache/maven/archiva/webdav/
	test/resources/
From: Deng Ching <oching@apache.org>
To: dev@archiva.apache.org
Content-Type: multipart/alternative; boundary=0050450292030c48d40475f3f321

--0050450292030c48d40475f3f321
Content-Type: text/plain; charset=ISO-8859-1

On Wed, Oct 14, 2009 at 11:40 PM, Brett Porter <brett@apache.org> wrote:

>
>
>>>       // MRM-872 : merge all available metadata
>>>
>>>>      // merge metadata only when requested via the repo group
>>>> -        if ( ( repositoryRequest.isMetadata( requestedResource ) || (
>>>> requestedResource.endsWith( "metadata.xml.sha1" ) ||
>>>> requestedResource.endsWith( "metadata.xml.md5" ) ) )
>>>> -            && repoGroupConfig != null )
>>>> +        if ( ( repositoryRequest.isMetadata( requestedResource ) || (
>>>> requestedResource.endsWith( "metadata.xml.sha1" ) ||
>>>> requestedResource.endsWith( "metadata.xml.md5" ) ) ) &&
>>>> +            repoGroupConfig != null )
>>>>
>>>>
>>> Should this use "isSupportFile" like below? That will cover the two
>>> metadata checksums
>>>
>>
>>
>> .. but it will also get the other non-metadata checksum files so I don't
>> think we can use isSupportFile(..) here
>>
>>
> ok - could the check be moved to the repository request (eg,
> isMetadataSupportFile), so that it is all in one spot?


ok, will move this one over..


>
>
>
>>
>>>
>>> @@ -482,6 +496,35 @@
>>>
>>>>
>>>>          if ( request.getMethod().equals( HTTP_PUT_METHOD ) )
>>>>          {
>>>> +                String resourcePath = logicalResource.getPath();
>>>> +
>>>> +                // check if target repo is enabled for releases
>>>> +                // we suppose that release-artifacts can deployed only
>>>> to
>>>> repos enabled for releases
>>>> +                if ( managedRepository.getRepository().isReleases() &&
>>>> !repositoryRequest.isMetadata( resourcePath ) &&
>>>> +                    !repositoryRequest.isSupportFile( resourcePath ) )
>>>> +                {
>>>> +                    ArtifactReference artifact = null;
>>>> +                    try
>>>> +                    {
>>>> +                        artifact =
>>>> managedRepository.toArtifactReference(
>>>> resourcePath );
>>>> +                    }
>>>> +                    catch ( LayoutException e )
>>>> +                    {
>>>> +                        throw new DavException(
>>>> HttpServletResponse.SC_BAD_REQUEST, e );
>>>> +                    }
>>>> +
>>>> +                    if ( !VersionUtil.isSnapshot( artifact.getVersion()
>>>> )
>>>> )
>>>> +                    {
>>>> +                        // check if artifact already exists
>>>> +                        if ( managedRepository.hasContent( artifact ) )
>>>> +                        {
>>>> +                            log.warn( "Overwriting released artifacts
>>>> is
>>>> not allowed." );
>>>> +                            throw new
>>>> ReleaseArtifactAlreadyExistsException( managedRepository.getId(),
>>>> +
>>>>   "Overwriting released artifacts is not allowed." );
>>>> +                        }
>>>> +                    }
>>>> +                }
>>>> +
>>>>
>>>>
>>> Is it necessarily a bad request if the reference can't be derived, or
>>> should the check just be skipped?
>>>
>>>
>> I don't think this is just a check though but it's for getting the
>> artifact
>> object and its coordinates. Maybe we could add a fall back for getting the
>> artifact obj & its coordinates when a LayoutException is thrown instead of
>> immediately propagating it as a bad request error?
>>
>
> The check I meant was the VersionUtil bit.
>
> Say you are trying to store /foo.txt using webdav, which is not a valid
> artifact. I believe this was still allowed previously (though I might be
> wrong) - but now it will be a bad request because of the artifact path, when
> all it needs that for is to check if it is a release. Does that make sense?


Ok, I get it now :) I think we still need to check if the version is a
SNAPSHOT or not though, and block deployment if it is a released artifact or
an invalid artifact (LayoutException is thrown) as long as it already exists
in the repository. We're also using the artifact reference to check if it is
in the repository so it's not just the version we're using. I'll see if
there's another way to do this.

Thanks,
Deng

--0050450292030c48d40475f3f321--