On 14/10/2009, at 7:03 PM, Deng Ching wrote:
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
ok - could the check be moved to the repository request (eg,
isMetadataSupportFile), so that it is all in one spot?
@@ -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?
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
Thanks!
- Brett
