Hi Brett, On Tue, Oct 13, 2009 at 8:49 PM, Brett Porter <[email protected]> wrote:
> > On 13/10/2009, at 9:36 PM, [email protected] 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 > >
