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 Wed, 14 Oct 2009 08:03:00 GMT
Hi Brett,

On Tue, Oct 13, 2009 at 8:49 PM, Brett Porter <brett@apache.org> wrote:

>
> On 13/10/2009, at 9:36 PM, oching@apache.org wrote:
>
>  -                resource =
>> -                    processRepositoryGroup( request, archivaLocator,
>> repoGroupConfig.getRepositories(),
>> -                                            activePrincipal,
>> resourcesInAbsolutePath );
>> +                try
>> +                {
>> +                    resource =
>> +                        processRepositoryGroup( request, archivaLocator,
>> repoGroupConfig.getRepositories(),
>> +                                                activePrincipal,
>> resourcesInAbsolutePath );
>> +                }
>> +                catch ( ReleaseArtifactAlreadyExistsException e )
>> +                {
>> +                    throw new DavException(
>> HttpServletResponse.SC_CONFLICT );
>> +                }
>>
>
>
> it might make more sense just to throw this at the source and eliminate the
> exception, since the result is always the same?


agreed :)


>
>         // 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


>
>
>  @@ -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?


> Also, given this is a point release (1.2.3), I don't think this kind of
> functionality change should be imposed on users - can we offer a
> configuration option?
>

+1
I've left the issue open yesterday since this functionality also needs to be
applied to the web upload.

Thanks,
Deng


> Cheers,
> Brett
>
>

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