archiva-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Deng Ching <och...@apache.org>
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/
Date Thu, 15 Oct 2009 07:08:41 GMT
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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message