You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/12/16 16:50:04 UTC

[GitHub] [maven-resolver] cstamas opened a new pull request #141: [MRESOLVER-231] Make extension point where checksums are injected.

cstamas opened a new pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141


   This feature allows Resolver to get "expected" checksums ahead
   of remote transport (resolution), hence, it may operate with
   "good known" checksums for example (or use any other source).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] cstamas merged pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
cstamas merged pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] michael-o commented on a change in pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#discussion_r795200766



##########
File path: maven-resolver-connector-basic/src/main/java/org/eclipse/aether/connector/basic/BasicRepositoryConnectorFactory.java
##########
@@ -132,6 +143,22 @@ public BasicRepositoryConnectorFactory setFileProcessor( FileProcessor fileProce
         return this;
     }
 
+    /**
+     * Sets the provided checksum sources to use for this component.
+     *
+     * @param providedChecksumsSources The provided checksum sources to use, must not be {@code null}.
+     * @return This component for chaining, never {@code null}.
+     * @since TBD
+     */
+    public BasicRepositoryConnectorFactory setProvidedChecksumSources(
+        Map<String, ProvidedChecksumsSource> providedChecksumsSources )
+    {
+        this.providedChecksumsSources = requireNonNull(
+            providedChecksumsSources, "provided checksum sources cannot be null "

Review comment:
       Trailing space

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/impl/guice/AetherModule.java
##########
@@ -207,6 +212,17 @@ protected void configure()
 
     }
 
+    @Provides
+    @Singleton
+    Map<String, ProvidedChecksumsSource> provideChecksumSources(
+        @Named( FileProvidedChecksumSource.NAME ) ProvidedChecksumsSource fileDownloadChecksumSource
+    )
+    {
+        Map<String, ProvidedChecksumsSource> downloadChecksumSources = new HashMap<>();

Review comment:
       Same here

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/impl/guice/AetherModule.java
##########
@@ -207,6 +212,17 @@ protected void configure()
 
     }
 
+    @Provides
+    @Singleton
+    Map<String, ProvidedChecksumsSource> provideChecksumSources(
+        @Named( FileProvidedChecksumSource.NAME ) ProvidedChecksumsSource fileDownloadChecksumSource

Review comment:
       Is `fileDownloadChecksumSource` still correct?

##########
File path: maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/checksum/ChecksumPolicy.java
##########
@@ -64,53 +64,71 @@
  */
 public interface ChecksumPolicy
 {
-
     /**
-     * Bit flag indicating a checksum which is not part of the official repository layout/structure.
+     * Enum denoting origin of checksum.
+     *
+     * @since TBD
      */
-    int KIND_UNOFFICIAL = 0x01;
+    enum ChecksumKind
+    {
+        /**
+         * Remote external kind of checksum are retrieved from remote doing extra transport round-trip (usually by
+         * getting "file.jar.sha1" for corresponding "file.jar" file). This kind of checksum is part of layout, and
+         * was from beginning the "official" (and one and only) checksum used by resolver.
+         */
+        REMOTE_EXTERNAL,
+
+        /**
+         * Included checksums may be received from remote repository during the retrieval of the main file, for example
+         * from response headers in case of HTTP transport. They may be set with
+         * {@link org.eclipse.aether.spi.connector.transport.GetTask#setChecksum(String, String)}. These are also
+         * known as "smart checksums".

Review comment:
       I wouldn't really use the term "smart chksm" in a new documentation because there is nothing smart about it.

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/FileProvidedChecksumSource.java
##########
@@ -0,0 +1,193 @@
+package org.eclipse.aether.internal.impl;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.spi.connector.ArtifactDownload;
+import org.eclipse.aether.spi.connector.MetadataDownload;
+import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
+import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
+import org.eclipse.aether.spi.io.FileProcessor;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Local filesystem backed {@link ProvidedChecksumsSource} implementation that use specified directory as base
+ * directory, where it expects artifacts checksums on standard Maven2 "local" layout. This implementation uses Artifact
+ * (and Metadata) coordinates solely to form path from baseDir (for Metadata file name is
+ * {@code maven-metadata-local.xml.sha1} in case of SHA-1 checksum).
+ *
+ * @since TBD
+ */
+@Singleton
+@Named( FileProvidedChecksumSource.NAME )
+public final class FileProvidedChecksumSource
+    implements ProvidedChecksumsSource
+{
+    public static final String NAME = "file";
+
+    static final String CONFIG_PROP_BASE_DIR = "aether.artifactResolver.providedChecksumSource.file.baseDir";

Review comment:
       `providedChecksumsSource`

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/FileProvidedChecksumSource.java
##########
@@ -0,0 +1,193 @@
+package org.eclipse.aether.internal.impl;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.spi.connector.ArtifactDownload;
+import org.eclipse.aether.spi.connector.MetadataDownload;
+import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
+import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
+import org.eclipse.aether.spi.io.FileProcessor;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Local filesystem backed {@link ProvidedChecksumsSource} implementation that use specified directory as base
+ * directory, where it expects artifacts checksums on standard Maven2 "local" layout. This implementation uses Artifact
+ * (and Metadata) coordinates solely to form path from baseDir (for Metadata file name is
+ * {@code maven-metadata-local.xml.sha1} in case of SHA-1 checksum).

Review comment:
       Isn't this rather `{repoId}` depending where the data came from?

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/FileProvidedChecksumSource.java
##########
@@ -0,0 +1,193 @@
+package org.eclipse.aether.internal.impl;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.spi.connector.ArtifactDownload;
+import org.eclipse.aether.spi.connector.MetadataDownload;
+import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
+import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
+import org.eclipse.aether.spi.io.FileProcessor;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Local filesystem backed {@link ProvidedChecksumsSource} implementation that use specified directory as base
+ * directory, where it expects artifacts checksums on standard Maven2 "local" layout. This implementation uses Artifact
+ * (and Metadata) coordinates solely to form path from baseDir (for Metadata file name is
+ * {@code maven-metadata-local.xml.sha1} in case of SHA-1 checksum).
+ *
+ * @since TBD
+ */
+@Singleton
+@Named( FileProvidedChecksumSource.NAME )
+public final class FileProvidedChecksumSource
+    implements ProvidedChecksumsSource
+{
+    public static final String NAME = "file";
+
+    static final String CONFIG_PROP_BASE_DIR = "aether.artifactResolver.providedChecksumSource.file.baseDir";
+
+    static final String LOCAL_REPO_PREFIX = ".checksums";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( FileProvidedChecksumSource.class );
+
+    private final FileProcessor fileProcessor;
+
+    private final SimpleLocalRepositoryManager simpleLocalRepositoryManager;
+
+    @Inject
+    public FileProvidedChecksumSource( FileProcessor fileProcessor )
+    {
+        this.fileProcessor = requireNonNull( fileProcessor );
+        // we really needs just "local layout" from it (relative paths), so baseDir here is irrelevant
+        this.simpleLocalRepositoryManager = new SimpleLocalRepositoryManager( new File( "" ) );
+    }
+
+    @Override
+    public Map<String, String> getProvidedMetadataChecksums( RepositorySystemSession session,
+                                                             MetadataDownload transfer,
+                                                             List<ChecksumAlgorithmFactory> checksumAlgorithmFactories )
+    {
+        Path baseDir = getBaseDir( session );
+        if ( baseDir == null )
+        {
+            return null;
+        }
+        ArrayList<ChecksumLocation> checksumLocations = new ArrayList<>( checksumAlgorithmFactories.size() );
+        for ( ChecksumAlgorithmFactory checksumAlgorithmFactory : checksumAlgorithmFactories )
+        {
+            checksumLocations.add( new ChecksumLocation(

Review comment:
       Hmm, I confused this `ChecksumLocation` with the public one...

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/FileProvidedChecksumSource.java
##########
@@ -0,0 +1,193 @@
+package org.eclipse.aether.internal.impl;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.spi.connector.ArtifactDownload;
+import org.eclipse.aether.spi.connector.MetadataDownload;
+import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
+import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
+import org.eclipse.aether.spi.io.FileProcessor;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Local filesystem backed {@link ProvidedChecksumsSource} implementation that use specified directory as base
+ * directory, where it expects artifacts checksums on standard Maven2 "local" layout. This implementation uses Artifact
+ * (and Metadata) coordinates solely to form path from baseDir (for Metadata file name is
+ * {@code maven-metadata-local.xml.sha1} in case of SHA-1 checksum).
+ *
+ * @since TBD
+ */
+@Singleton
+@Named( FileProvidedChecksumSource.NAME )
+public final class FileProvidedChecksumSource

Review comment:
       `FileProvidedChecksumsSource`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] cstamas commented on a change in pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#discussion_r795066419



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/FileProvidedChecksumSource.java
##########
@@ -0,0 +1,188 @@
+package org.eclipse.aether.internal.impl;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.spi.connector.ArtifactDownload;
+import org.eclipse.aether.spi.connector.MetadataDownload;
+import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
+import org.eclipse.aether.spi.connector.layout.RepositoryLayout;
+import org.eclipse.aether.spi.connector.layout.RepositoryLayout.ChecksumLocation;
+import org.eclipse.aether.spi.io.FileProcessor;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * {@link ProvidedChecksumsSource} that does use specified directory structure to look up checksums.
+ *
+ * @since TBD
+ */
+@Singleton
+@Named( FileProvidedChecksumSource.NAME )
+public final class FileProvidedChecksumSource
+    implements ProvidedChecksumsSource
+{
+    public static final String NAME = "file";
+
+    static final String CONFIG_PROP_BASE_DIR = "aether.artifactResolver.providedChecksumSource.file.baseDir";
+
+    static final String LOCAL_REPO_PREFIX = ".checksums";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( FileProvidedChecksumSource.class );
+
+    private final FileProcessor fileProcessor;
+
+    @Inject
+    public FileProvidedChecksumSource( FileProcessor fileProcessor )
+    {
+        this.fileProcessor = requireNonNull( fileProcessor );
+    }
+
+    @Override
+    public Map<String, String> getProvidedMetadataChecksums( RepositorySystemSession session,
+                                                             MetadataDownload transfer,
+                                                             RepositoryLayout repositoryLayout )
+    {
+        URI baseURI = getBaseURI( session );
+        if ( baseURI == null )
+        {
+            return null;
+        }
+
+        HashMap<String, String> checksums = new HashMap<>();
+        Metadata metadata = transfer.getMetadata();
+        List<ChecksumLocation> checksumLocations = repositoryLayout.getChecksumLocations(
+            metadata, false, repositoryLayout.getLocation( metadata, false )
+        );
+        for ( ChecksumLocation checksumLocation : checksumLocations )
+        {
+            Path checksumPath = new File( baseURI.resolve( checksumLocation.getLocation() ) ).toPath();
+            if ( Files.isReadable( checksumPath ) )
+            {
+                try
+                {
+                    String checksum = fileProcessor.readChecksum( checksumPath.toFile() );
+                    if ( checksum != null )
+                    {
+                        if ( LOGGER.isDebugEnabled() )
+                        {
+                            LOGGER.debug( "{} -> {}:{}", metadata,
+                                checksumLocation.getChecksumAlgorithmFactory().getName(), checksum );
+                        }
+
+                        checksums.put( checksumLocation.getChecksumAlgorithmFactory().getName(), checksum );
+                    }
+                }
+                catch ( IOException e )
+                {
+                    LOGGER.warn( "Could not read provided checksum for {} at path {}", metadata, checksumPath, e );
+                }
+            }
+        }
+        return checksums.isEmpty() ? null : checksums;
+    }
+
+    @Override
+    public Map<String, String> getProvidedArtifactChecksums( RepositorySystemSession session,
+                                                             ArtifactDownload transfer,
+                                                             RepositoryLayout repositoryLayout )
+    {
+        URI baseURI = getBaseURI( session );
+        if ( baseURI == null )
+        {
+            return null;
+        }
+
+        HashMap<String, String> checksums = new HashMap<>();
+        Artifact artifact = transfer.getArtifact();
+        List<ChecksumLocation> checksumLocations = repositoryLayout.getChecksumLocations(
+            artifact, false, repositoryLayout.getLocation( artifact, false )
+        );
+        for ( ChecksumLocation checksumLocation : checksumLocations )
+        {
+            Path checksumPath = new File( baseURI.resolve( checksumLocation.getLocation() ) ).toPath();
+            if ( Files.isReadable( checksumPath ) )
+            {
+                try
+                {
+                    String checksum = fileProcessor.readChecksum( checksumPath.toFile() );
+                    if ( checksum != null )
+                    {
+                        if ( LOGGER.isDebugEnabled() )
+                        {
+                            LOGGER.debug( "{} -> {}:{}", ArtifactIdUtils.toId( artifact ),

Review comment:
       There is no `ArtifactUtils.toId( Metadata.class )`...




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] cstamas commented on a change in pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#discussion_r795066332



##########
File path: maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/checksum/ChecksumPolicy.java
##########
@@ -64,53 +64,52 @@
  */
 public interface ChecksumPolicy
 {
-
     /**
-     * Bit flag indicating a checksum which is not part of the official repository layout/structure.
+     * Enum denoting origin of checksum: remotely fetched (was before the "official"), smart or provided.
+     *
+     * @since TBD
      */
-    int KIND_UNOFFICIAL = 0x01;
+    enum ChecksumKind
+    {
+        REMOTE, SMART, PROVIDED

Review comment:
       fixed




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] michael-o commented on a change in pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#discussion_r795477166



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/AbstractChecksumPolicy.java
##########
@@ -41,26 +41,26 @@ protected AbstractChecksumPolicy( TransferResource resource )
     }
 
     @Override
-    public boolean onChecksumMatch( String algorithm, int kind )
+    public boolean onChecksumMatch( String algorithm, ChecksumKind kind )
     {
         requireNonNull( algorithm, "algorithm cannot be null" );
         return true;
     }
 
     @Override
-    public void onChecksumMismatch( String algorithm, int kind, ChecksumFailureException exception )
+    public void onChecksumMismatch( String algorithm, ChecksumKind kind, ChecksumFailureException exception )
             throws ChecksumFailureException
     {
         requireNonNull( algorithm, "algorithm cannot be null" );
         requireNonNull( exception, "exception cannot be null" );
-        if ( ( kind & KIND_UNOFFICIAL ) == 0 )
+        if ( !kind.isIgnoreOnMismatch() )

Review comment:
       I'd drop this altogether. We have a checksum policy. Not the checksum kind should decide this.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] michael-o commented on a change in pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#discussion_r795236549



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/FileProvidedChecksumSource.java
##########
@@ -0,0 +1,193 @@
+package org.eclipse.aether.internal.impl;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.spi.connector.ArtifactDownload;
+import org.eclipse.aether.spi.connector.MetadataDownload;
+import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
+import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
+import org.eclipse.aether.spi.io.FileProcessor;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Local filesystem backed {@link ProvidedChecksumsSource} implementation that use specified directory as base
+ * directory, where it expects artifacts checksums on standard Maven2 "local" layout. This implementation uses Artifact
+ * (and Metadata) coordinates solely to form path from baseDir (for Metadata file name is
+ * {@code maven-metadata-local.xml.sha1} in case of SHA-1 checksum).

Review comment:
       there is no checksum for a local file:
   ```
   $ find . -name maven-metadata\*.xml.\* -type f -exec basename {} \; | sort -u
   maven-metadata-apache.snapshots.https.xml.sha1
   maven-metadata-apache.snapshots.xml.sha1
   maven-metadata-central-mirror.xml.sha1
   maven-metadata-central.xml.sha1
   maven-metadata-dynamowerk-public.xml.sha1
   maven-metadata-jboss.release.xml.sha1
   maven-metadata-lda-public.xml.sha1
   maven-metadata-maven-core-it.xml.sha1
   maven-metadata-nexus-dynamowerk-berlin.xml.sha1
   maven-metadata-nexus-dynamowerk-berlin.xml.sha256
   maven-metadata-nexus-dynamowerk-berlin.xml.sha512
   maven-metadata-public.xml.sha1
   maven-metadata-repository.jboss.org.xml.sha1
   maven-metadata-snapshot-repository.xml.sha1
   maven-metadata-sonatype-nexus-snapshots.xml.sha1
   maven-metadata-sonatype.xml.sha1
   maven-metadata-tesla.xml.part
   maven-metadata-tesla.xml.part.lock
   ```
   and
   ```
   $ find . -name maven-metadata\*.xml -type f -exec basename {} \; | sort -u
   maven-metadata-apache.snapshots.https.xml
   maven-metadata-apache.snapshots.xml
   maven-metadata-central-mirror.xml
   maven-metadata-central.xml
   maven-metadata-dynamowerk-public.xml
   maven-metadata-jboss.release.xml
   maven-metadata-lda-public.xml
   maven-metadata-lda-releases.xml
   maven-metadata-local.xml
   maven-metadata-maven-core-it.xml
   maven-metadata-nexus-dynamowerk-berlin.xml
   maven-metadata-public.xml
   maven-metadata-remote.xml
   maven-metadata-repository.jboss.org.xml
   maven-metadata-snapshot-repository.xml
   maven-metadata-sonatype-nexus-snapshots.xml
   maven-metadata-sonatype.xml
   ```
   
   As far as I understand, the checksum is for a remote repo and the checksum will change as soon as the metadata will change. An artifact is immutable, but the metadata is not. So you will need to update the metadata checksum everytime. I think this does not make sense. I think we should drop this.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] cstamas commented on pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#issuecomment-1025503386


   Tested on https://github.com/apache/maven/pull/635
   
   Console snippets:
   ```
   [WARNING] Checksum validation failed, expected BAD but is cf43b5391de623b36fe066a21127baef82c64022 from walmart-grid for http://urnebes.local:8881/content/groups/wlabs/org/codehaus/plexus/plexus-utils/3.3.0/plexus-utils-3.3.0.jar
   ...
   [INFO] --------------------------------------------------------------------------------------------------------------------------
   [INFO] BUILD FAILURE
   [INFO] --------------------------------------------------------------------------------------------------------------------------
   [INFO] Total time:  1.811 s
   [INFO] Finished at: 2022-01-31T09:47:54+01:00
   [INFO] --------------------------------------------------------------------------------------------------------------------------
   [ERROR] Failed to execute goal org.apache.maven.plugins:maven-enforcer-plugin:3.0.0:enforce (enforce-maven-version) on project maven-bom: Execution enforce-maven-version of goal org.apache.maven.plugins:maven-enforcer-plugin:3.0.0:enforce failed: Plugin org.apache.maven.plugins:maven-enforcer-plugin:3.0.0 or one of its dependencies could not be resolved: Could not transfer artifact org.codehaus.plexus:plexus-utils:jar:3.3.0 from/to walmart-grid (http://urnebes.local:8881/content/groups/wlabs/): Checksum validation failed, expected BAD but is cf43b5391de623b36fe066a21127baef82c64022 -> [Help 1]
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] michael-o commented on pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#issuecomment-1025589418


   MAke sure to replace TBD with 1.8.0


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] michael-o commented on a change in pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#discussion_r795242905



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/FileProvidedChecksumsSource.java
##########
@@ -0,0 +1,193 @@
+package org.eclipse.aether.internal.impl;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.spi.connector.ArtifactDownload;
+import org.eclipse.aether.spi.connector.MetadataDownload;
+import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
+import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
+import org.eclipse.aether.spi.io.FileProcessor;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Local filesystem backed {@link ProvidedChecksumsSource} implementation that use specified directory as base
+ * directory, where it expects artifacts checksums on standard Maven2 "local" layout. This implementation uses Artifact
+ * (and Metadata) coordinates solely to form path from baseDir (for Metadata file name is
+ * {@code maven-metadata-local.xml.sha1} in case of SHA-1 checksum).
+ *
+ * @since TBD
+ */
+@Singleton
+@Named( FileProvidedChecksumsSource.NAME )
+public final class FileProvidedChecksumsSource
+    implements ProvidedChecksumsSource
+{
+    public static final String NAME = "file";
+
+    static final String CONFIG_PROP_BASE_DIR = "aether.artifactResolver.providedChecksumsSource.file.baseDir";
+
+    static final String LOCAL_REPO_PREFIX = ".checksums";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( FileProvidedChecksumsSource.class );
+
+    private final FileProcessor fileProcessor;
+
+    private final SimpleLocalRepositoryManager simpleLocalRepositoryManager;
+
+    @Inject
+    public FileProvidedChecksumsSource( FileProcessor fileProcessor )
+    {
+        this.fileProcessor = requireNonNull( fileProcessor );
+        // we really needs just "local layout" from it (relative paths), so baseDir here is irrelevant
+        this.simpleLocalRepositoryManager = new SimpleLocalRepositoryManager( new File( "" ) );
+    }
+
+    @Override
+    public Map<String, String> getProvidedMetadataChecksums( RepositorySystemSession session,
+                                                             MetadataDownload transfer,
+                                                             List<ChecksumAlgorithmFactory> checksumAlgorithmFactories )
+    {
+        Path baseDir = getBaseDir( session );
+        if ( baseDir == null )
+        {
+            return null;
+        }
+        ArrayList<ChecksumFilePath> checksumFilePaths = new ArrayList<>( checksumAlgorithmFactories.size() );
+        for ( ChecksumAlgorithmFactory checksumAlgorithmFactory : checksumAlgorithmFactories )
+        {
+            checksumFilePaths.add( new ChecksumFilePath(
+                    simpleLocalRepositoryManager.getPathForLocalMetadata( transfer.getMetadata() ) + '.'
+                    + checksumAlgorithmFactory.getFileExtension(), checksumAlgorithmFactory ) );
+        }
+        return getProvidedChecksums( baseDir, checksumFilePaths, String.valueOf( transfer.getMetadata() ) );
+    }
+
+    @Override
+    public Map<String, String> getProvidedArtifactChecksums( RepositorySystemSession session,
+                                                             ArtifactDownload transfer,
+                                                             List<ChecksumAlgorithmFactory> checksumAlgorithmFactories )
+    {
+        Path baseDir = getBaseDir( session );
+        if ( baseDir == null )
+        {
+            return null;
+        }
+        ArrayList<ChecksumFilePath> checksumFilePaths = new ArrayList<>( checksumAlgorithmFactories.size() );
+        for ( ChecksumAlgorithmFactory checksumAlgorithmFactory : checksumAlgorithmFactories )
+        {
+            checksumFilePaths.add( new ChecksumFilePath(
+                    simpleLocalRepositoryManager.getPathForArtifact( transfer.getArtifact(), false ) + '.'
+                    + checksumAlgorithmFactory.getFileExtension(), checksumAlgorithmFactory ) );
+        }
+        return getProvidedChecksums( baseDir, checksumFilePaths, ArtifactIdUtils.toId( transfer.getArtifact() ) );
+    }
+
+    /**
+     * May return {@code null}.
+     */
+    private Map<String, String> getProvidedChecksums( Path baseDir,
+                                                      List<ChecksumFilePath> checksumFilePaths,
+                                                      String subjectId )
+    {
+        HashMap<String, String> checksums = new HashMap<>();
+        for ( ChecksumFilePath checksumFilePath : checksumFilePaths )
+        {
+            Path checksumPath =  baseDir.resolve( checksumFilePath.path );
+            if ( Files.isReadable( checksumPath ) )
+            {
+                try
+                {
+                    String checksum = fileProcessor.readChecksum( checksumPath.toFile() );
+                    if ( checksum != null )
+                    {
+                        if ( LOGGER.isDebugEnabled() )

Review comment:
       This is redudant with SLF4J.

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/FileProvidedChecksumsSource.java
##########
@@ -0,0 +1,193 @@
+package org.eclipse.aether.internal.impl;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.spi.connector.ArtifactDownload;
+import org.eclipse.aether.spi.connector.MetadataDownload;
+import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
+import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
+import org.eclipse.aether.spi.io.FileProcessor;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Local filesystem backed {@link ProvidedChecksumsSource} implementation that use specified directory as base
+ * directory, where it expects artifacts checksums on standard Maven2 "local" layout. This implementation uses Artifact
+ * (and Metadata) coordinates solely to form path from baseDir (for Metadata file name is
+ * {@code maven-metadata-local.xml.sha1} in case of SHA-1 checksum).
+ *
+ * @since TBD
+ */
+@Singleton
+@Named( FileProvidedChecksumsSource.NAME )
+public final class FileProvidedChecksumsSource
+    implements ProvidedChecksumsSource
+{
+    public static final String NAME = "file";
+
+    static final String CONFIG_PROP_BASE_DIR = "aether.artifactResolver.providedChecksumsSource.file.baseDir";
+
+    static final String LOCAL_REPO_PREFIX = ".checksums";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( FileProvidedChecksumsSource.class );
+
+    private final FileProcessor fileProcessor;
+
+    private final SimpleLocalRepositoryManager simpleLocalRepositoryManager;
+
+    @Inject
+    public FileProvidedChecksumsSource( FileProcessor fileProcessor )
+    {
+        this.fileProcessor = requireNonNull( fileProcessor );
+        // we really needs just "local layout" from it (relative paths), so baseDir here is irrelevant
+        this.simpleLocalRepositoryManager = new SimpleLocalRepositoryManager( new File( "" ) );
+    }
+
+    @Override
+    public Map<String, String> getProvidedMetadataChecksums( RepositorySystemSession session,
+                                                             MetadataDownload transfer,
+                                                             List<ChecksumAlgorithmFactory> checksumAlgorithmFactories )
+    {
+        Path baseDir = getBaseDir( session );
+        if ( baseDir == null )
+        {
+            return null;
+        }
+        ArrayList<ChecksumFilePath> checksumFilePaths = new ArrayList<>( checksumAlgorithmFactories.size() );
+        for ( ChecksumAlgorithmFactory checksumAlgorithmFactory : checksumAlgorithmFactories )
+        {
+            checksumFilePaths.add( new ChecksumFilePath(
+                    simpleLocalRepositoryManager.getPathForLocalMetadata( transfer.getMetadata() ) + '.'
+                    + checksumAlgorithmFactory.getFileExtension(), checksumAlgorithmFactory ) );
+        }
+        return getProvidedChecksums( baseDir, checksumFilePaths, String.valueOf( transfer.getMetadata() ) );
+    }
+
+    @Override
+    public Map<String, String> getProvidedArtifactChecksums( RepositorySystemSession session,
+                                                             ArtifactDownload transfer,
+                                                             List<ChecksumAlgorithmFactory> checksumAlgorithmFactories )
+    {
+        Path baseDir = getBaseDir( session );
+        if ( baseDir == null )
+        {
+            return null;
+        }
+        ArrayList<ChecksumFilePath> checksumFilePaths = new ArrayList<>( checksumAlgorithmFactories.size() );
+        for ( ChecksumAlgorithmFactory checksumAlgorithmFactory : checksumAlgorithmFactories )
+        {
+            checksumFilePaths.add( new ChecksumFilePath(
+                    simpleLocalRepositoryManager.getPathForArtifact( transfer.getArtifact(), false ) + '.'
+                    + checksumAlgorithmFactory.getFileExtension(), checksumAlgorithmFactory ) );
+        }
+        return getProvidedChecksums( baseDir, checksumFilePaths, ArtifactIdUtils.toId( transfer.getArtifact() ) );
+    }
+
+    /**
+     * May return {@code null}.
+     */
+    private Map<String, String> getProvidedChecksums( Path baseDir,
+                                                      List<ChecksumFilePath> checksumFilePaths,
+                                                      String subjectId )
+    {
+        HashMap<String, String> checksums = new HashMap<>();
+        for ( ChecksumFilePath checksumFilePath : checksumFilePaths )
+        {
+            Path checksumPath =  baseDir.resolve( checksumFilePath.path );
+            if ( Files.isReadable( checksumPath ) )
+            {
+                try
+                {
+                    String checksum = fileProcessor.readChecksum( checksumPath.toFile() );
+                    if ( checksum != null )
+                    {
+                        if ( LOGGER.isDebugEnabled() )
+                        {
+                            LOGGER.debug( "{} -> {}:{}", subjectId,
+                                    checksumFilePath.checksumAlgorithmFactory.getName(), checksum );

Review comment:
       I think even for debugging purposes this needs more text. E.g.:
   
   > "Resolved provided checksum '{}:{}' for '{}'"




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] michael-o commented on pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#issuecomment-1024970622


   Crunching on inline and external. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] michael-o commented on a change in pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#discussion_r795468773



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/AbstractChecksumPolicy.java
##########
@@ -41,26 +41,26 @@ protected AbstractChecksumPolicy( TransferResource resource )
     }
 
     @Override
-    public boolean onChecksumMatch( String algorithm, int kind )
+    public boolean onChecksumMatch( String algorithm, ChecksumKind kind )
     {
         requireNonNull( algorithm, "algorithm cannot be null" );
         return true;
     }
 
     @Override
-    public void onChecksumMismatch( String algorithm, int kind, ChecksumFailureException exception )
+    public void onChecksumMismatch( String algorithm, ChecksumKind kind, ChecksumFailureException exception )
             throws ChecksumFailureException
     {
         requireNonNull( algorithm, "algorithm cannot be null" );
         requireNonNull( exception, "exception cannot be null" );
-        if ( ( kind & KIND_UNOFFICIAL ) == 0 )
+        if ( !kind.isIgnoreOnMismatch() )

Review comment:
       This isn't good. We have a clear checksum policy and this defeats the policy. The source of the checksums should be irrelevant. `REMOTE` and `PROVIDED` bust be equal.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] cstamas commented on pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#issuecomment-1024996602


   Meh, it's again site/javadoc that fails.... 😞 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] cstamas commented on a change in pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#discussion_r795211628



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/FileProvidedChecksumSource.java
##########
@@ -0,0 +1,193 @@
+package org.eclipse.aether.internal.impl;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.spi.connector.ArtifactDownload;
+import org.eclipse.aether.spi.connector.MetadataDownload;
+import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
+import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
+import org.eclipse.aether.spi.io.FileProcessor;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Local filesystem backed {@link ProvidedChecksumsSource} implementation that use specified directory as base
+ * directory, where it expects artifacts checksums on standard Maven2 "local" layout. This implementation uses Artifact
+ * (and Metadata) coordinates solely to form path from baseDir (for Metadata file name is
+ * {@code maven-metadata-local.xml.sha1} in case of SHA-1 checksum).

Review comment:
       Yes, but as I have to "misuse" SimpleLRM, I think this is acceptable compromise. Also, am still not 100% sure do we really want to provide checksums for metadata (!)... At worst, this branch will not be as much used as artifact branch




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] michael-o commented on pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#issuecomment-1024960505


   can you really make always the assumption that provided is always local?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] cstamas commented on pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#issuecomment-1024994622


   @michael-o while crunching, refactored and made it simpler, last two commits...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] cstamas commented on a change in pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#discussion_r795472526



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/AbstractChecksumPolicy.java
##########
@@ -41,26 +41,26 @@ protected AbstractChecksumPolicy( TransferResource resource )
     }
 
     @Override
-    public boolean onChecksumMatch( String algorithm, int kind )
+    public boolean onChecksumMatch( String algorithm, ChecksumKind kind )
     {
         requireNonNull( algorithm, "algorithm cannot be null" );
         return true;
     }
 
     @Override
-    public void onChecksumMismatch( String algorithm, int kind, ChecksumFailureException exception )
+    public void onChecksumMismatch( String algorithm, ChecksumKind kind, ChecksumFailureException exception )
             throws ChecksumFailureException
     {
         requireNonNull( algorithm, "algorithm cannot be null" );
         requireNonNull( exception, "exception cannot be null" );
-        if ( ( kind & KIND_UNOFFICIAL ) == 0 )
+        if ( !kind.isIgnoreOnMismatch() )

Review comment:
       Unsure do I get your comment: they are (they have same isIgnoreOnMismatch value -- so they are handled equally)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] cstamas commented on pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
cstamas commented on pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#issuecomment-1024967088


   > can you really make always the assumption that provided is always local?
   
   You are right here, so you propose to make it simply `PROVIDED` (remove the `LOCAL_` prefix)?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] michael-o commented on pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#issuecomment-1024970396


   > > can you really make always the assumption that provided is always local?
   > 
   > You are right here, so you propose to make it simply `PROVIDED` (remove the `LOCAL_` prefix)?
   
   Yes, what provided means is upto the implementor. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] cstamas commented on a change in pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#discussion_r795516215



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/AbstractChecksumPolicy.java
##########
@@ -41,26 +41,26 @@ protected AbstractChecksumPolicy( TransferResource resource )
     }
 
     @Override
-    public boolean onChecksumMatch( String algorithm, int kind )
+    public boolean onChecksumMatch( String algorithm, ChecksumKind kind )
     {
         requireNonNull( algorithm, "algorithm cannot be null" );
         return true;
     }
 
     @Override
-    public void onChecksumMismatch( String algorithm, int kind, ChecksumFailureException exception )
+    public void onChecksumMismatch( String algorithm, ChecksumKind kind, ChecksumFailureException exception )
             throws ChecksumFailureException
     {
         requireNonNull( algorithm, "algorithm cannot be null" );
         requireNonNull( exception, "exception cannot be null" );
-        if ( ( kind & KIND_UNOFFICIAL ) == 0 )
+        if ( !kind.isIgnoreOnMismatch() )

Review comment:
       https://issues.apache.org/jira/browse/MRESOLVER-237 coming next




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] michael-o commented on a change in pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#discussion_r795037478



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/FileProvidedChecksumSource.java
##########
@@ -0,0 +1,188 @@
+package org.eclipse.aether.internal.impl;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.spi.connector.ArtifactDownload;
+import org.eclipse.aether.spi.connector.MetadataDownload;
+import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
+import org.eclipse.aether.spi.connector.layout.RepositoryLayout;
+import org.eclipse.aether.spi.connector.layout.RepositoryLayout.ChecksumLocation;
+import org.eclipse.aether.spi.io.FileProcessor;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * {@link ProvidedChecksumsSource} that does use specified directory structure to look up checksums.
+ *
+ * @since TBD
+ */
+@Singleton
+@Named( FileProvidedChecksumSource.NAME )
+public final class FileProvidedChecksumSource
+    implements ProvidedChecksumsSource
+{
+    public static final String NAME = "file";
+
+    static final String CONFIG_PROP_BASE_DIR = "aether.artifactResolver.providedChecksumSource.file.baseDir";
+
+    static final String LOCAL_REPO_PREFIX = ".checksums";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( FileProvidedChecksumSource.class );
+
+    private final FileProcessor fileProcessor;
+
+    @Inject
+    public FileProvidedChecksumSource( FileProcessor fileProcessor )
+    {
+        this.fileProcessor = requireNonNull( fileProcessor );
+    }
+
+    @Override
+    public Map<String, String> getProvidedMetadataChecksums( RepositorySystemSession session,

Review comment:
       Both methods look almost identical, would it make sense to refactor the common code to a private method?

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/FileProvidedChecksumSource.java
##########
@@ -0,0 +1,188 @@
+package org.eclipse.aether.internal.impl;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.spi.connector.ArtifactDownload;
+import org.eclipse.aether.spi.connector.MetadataDownload;
+import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
+import org.eclipse.aether.spi.connector.layout.RepositoryLayout;
+import org.eclipse.aether.spi.connector.layout.RepositoryLayout.ChecksumLocation;
+import org.eclipse.aether.spi.io.FileProcessor;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * {@link ProvidedChecksumsSource} that does use specified directory structure to look up checksums.
+ *
+ * @since TBD
+ */
+@Singleton
+@Named( FileProvidedChecksumSource.NAME )
+public final class FileProvidedChecksumSource
+    implements ProvidedChecksumsSource
+{
+    public static final String NAME = "file";
+
+    static final String CONFIG_PROP_BASE_DIR = "aether.artifactResolver.providedChecksumSource.file.baseDir";
+
+    static final String LOCAL_REPO_PREFIX = ".checksums";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( FileProvidedChecksumSource.class );
+
+    private final FileProcessor fileProcessor;
+
+    @Inject
+    public FileProvidedChecksumSource( FileProcessor fileProcessor )
+    {
+        this.fileProcessor = requireNonNull( fileProcessor );
+    }
+
+    @Override
+    public Map<String, String> getProvidedMetadataChecksums( RepositorySystemSession session,
+                                                             MetadataDownload transfer,
+                                                             RepositoryLayout repositoryLayout )
+    {
+        URI baseURI = getBaseURI( session );
+        if ( baseURI == null )
+        {
+            return null;
+        }
+
+        HashMap<String, String> checksums = new HashMap<>();
+        Metadata metadata = transfer.getMetadata();
+        List<ChecksumLocation> checksumLocations = repositoryLayout.getChecksumLocations(
+            metadata, false, repositoryLayout.getLocation( metadata, false )
+        );
+        for ( ChecksumLocation checksumLocation : checksumLocations )
+        {
+            Path checksumPath = new File( baseURI.resolve( checksumLocation.getLocation() ) ).toPath();
+            if ( Files.isReadable( checksumPath ) )
+            {
+                try
+                {
+                    String checksum = fileProcessor.readChecksum( checksumPath.toFile() );
+                    if ( checksum != null )
+                    {
+                        if ( LOGGER.isDebugEnabled() )
+                        {
+                            LOGGER.debug( "{} -> {}:{}", metadata,
+                                checksumLocation.getChecksumAlgorithmFactory().getName(), checksum );
+                        }
+
+                        checksums.put( checksumLocation.getChecksumAlgorithmFactory().getName(), checksum );
+                    }
+                }
+                catch ( IOException e )
+                {
+                    LOGGER.warn( "Could not read provided checksum for {} at path {}", metadata, checksumPath, e );
+                }
+            }
+        }
+        return checksums.isEmpty() ? null : checksums;
+    }
+
+    @Override
+    public Map<String, String> getProvidedArtifactChecksums( RepositorySystemSession session,
+                                                             ArtifactDownload transfer,
+                                                             RepositoryLayout repositoryLayout )
+    {
+        URI baseURI = getBaseURI( session );
+        if ( baseURI == null )
+        {
+            return null;
+        }
+
+        HashMap<String, String> checksums = new HashMap<>();
+        Artifact artifact = transfer.getArtifact();
+        List<ChecksumLocation> checksumLocations = repositoryLayout.getChecksumLocations(
+            artifact, false, repositoryLayout.getLocation( artifact, false )
+        );
+        for ( ChecksumLocation checksumLocation : checksumLocations )
+        {
+            Path checksumPath = new File( baseURI.resolve( checksumLocation.getLocation() ) ).toPath();
+            if ( Files.isReadable( checksumPath ) )
+            {
+                try
+                {
+                    String checksum = fileProcessor.readChecksum( checksumPath.toFile() );
+                    if ( checksum != null )
+                    {
+                        if ( LOGGER.isDebugEnabled() )
+                        {
+                            LOGGER.debug( "{} -> {}:{}", ArtifactIdUtils.toId( artifact ),
+                                checksumLocation.getChecksumAlgorithmFactory().getName(), checksum );
+                        }
+
+                        checksums.put( checksumLocation.getChecksumAlgorithmFactory().getName(), checksum );
+                    }
+                }
+                catch ( IOException e )
+                {
+                    LOGGER.warn( "Could not read provided checksum for {} at path {}",

Review comment:
       Same here

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/FileProvidedChecksumSource.java
##########
@@ -0,0 +1,188 @@
+package org.eclipse.aether.internal.impl;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.spi.connector.ArtifactDownload;
+import org.eclipse.aether.spi.connector.MetadataDownload;
+import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
+import org.eclipse.aether.spi.connector.layout.RepositoryLayout;
+import org.eclipse.aether.spi.connector.layout.RepositoryLayout.ChecksumLocation;
+import org.eclipse.aether.spi.io.FileProcessor;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * {@link ProvidedChecksumsSource} that does use specified directory structure to look up checksums.
+ *
+ * @since TBD
+ */
+@Singleton
+@Named( FileProvidedChecksumSource.NAME )
+public final class FileProvidedChecksumSource
+    implements ProvidedChecksumsSource
+{
+    public static final String NAME = "file";
+
+    static final String CONFIG_PROP_BASE_DIR = "aether.artifactResolver.providedChecksumSource.file.baseDir";
+
+    static final String LOCAL_REPO_PREFIX = ".checksums";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( FileProvidedChecksumSource.class );
+
+    private final FileProcessor fileProcessor;
+
+    @Inject
+    public FileProvidedChecksumSource( FileProcessor fileProcessor )
+    {
+        this.fileProcessor = requireNonNull( fileProcessor );
+    }
+
+    @Override
+    public Map<String, String> getProvidedMetadataChecksums( RepositorySystemSession session,
+                                                             MetadataDownload transfer,
+                                                             RepositoryLayout repositoryLayout )
+    {
+        URI baseURI = getBaseURI( session );
+        if ( baseURI == null )
+        {
+            return null;
+        }
+
+        HashMap<String, String> checksums = new HashMap<>();
+        Metadata metadata = transfer.getMetadata();
+        List<ChecksumLocation> checksumLocations = repositoryLayout.getChecksumLocations(
+            metadata, false, repositoryLayout.getLocation( metadata, false )
+        );
+        for ( ChecksumLocation checksumLocation : checksumLocations )
+        {
+            Path checksumPath = new File( baseURI.resolve( checksumLocation.getLocation() ) ).toPath();
+            if ( Files.isReadable( checksumPath ) )
+            {
+                try
+                {
+                    String checksum = fileProcessor.readChecksum( checksumPath.toFile() );
+                    if ( checksum != null )
+                    {
+                        if ( LOGGER.isDebugEnabled() )
+                        {
+                            LOGGER.debug( "{} -> {}:{}", metadata,
+                                checksumLocation.getChecksumAlgorithmFactory().getName(), checksum );
+                        }
+
+                        checksums.put( checksumLocation.getChecksumAlgorithmFactory().getName(), checksum );
+                    }
+                }
+                catch ( IOException e )
+                {
+                    LOGGER.warn( "Could not read provided checksum for {} at path {}", metadata, checksumPath, e );

Review comment:
       Add single quotes to args.

##########
File path: maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/checksum/ChecksumPolicy.java
##########
@@ -64,53 +64,52 @@
  */
 public interface ChecksumPolicy
 {
-
     /**
-     * Bit flag indicating a checksum which is not part of the official repository layout/structure.
+     * Enum denoting origin of checksum: remotely fetched (was before the "official"), smart or provided.
+     *
+     * @since TBD
      */
-    int KIND_UNOFFICIAL = 0x01;
+    enum ChecksumKind
+    {
+        REMOTE, SMART, PROVIDED

Review comment:
       I absolutely dislike the term "smart" because computers aren't smart. They are stupid. The inline checksums are also remote. I would prefer some other term for it.

##########
File path: maven-resolver-connector-basic/src/main/java/org/eclipse/aether/connector/basic/ChecksumValidator.java
##########
@@ -97,7 +102,13 @@ public void validate( Map<String, ?> actualChecksums, Map<String, ?> inlinedChec
         {
             return;
         }
-        if ( inlinedChecksums != null && validateInlinedChecksums( actualChecksums, inlinedChecksums ) )
+        if ( providedChecksums != null
+               && validateChecksums( actualChecksums, ChecksumKind.PROVIDED, providedChecksums ) )
+        {
+            return;
+        }
+        if ( inlinedChecksums != null
+               && validateChecksums( actualChecksums, ChecksumKind.SMART, inlinedChecksums ) )

Review comment:
       `SMART` => `REMOTE_INLINE`?

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/FileProvidedChecksumSource.java
##########
@@ -0,0 +1,188 @@
+package org.eclipse.aether.internal.impl;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.spi.connector.ArtifactDownload;
+import org.eclipse.aether.spi.connector.MetadataDownload;
+import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
+import org.eclipse.aether.spi.connector.layout.RepositoryLayout;
+import org.eclipse.aether.spi.connector.layout.RepositoryLayout.ChecksumLocation;
+import org.eclipse.aether.spi.io.FileProcessor;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * {@link ProvidedChecksumsSource} that does use specified directory structure to look up checksums.
+ *
+ * @since TBD
+ */
+@Singleton
+@Named( FileProvidedChecksumSource.NAME )
+public final class FileProvidedChecksumSource
+    implements ProvidedChecksumsSource
+{
+    public static final String NAME = "file";
+
+    static final String CONFIG_PROP_BASE_DIR = "aether.artifactResolver.providedChecksumSource.file.baseDir";
+
+    static final String LOCAL_REPO_PREFIX = ".checksums";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( FileProvidedChecksumSource.class );
+
+    private final FileProcessor fileProcessor;
+
+    @Inject
+    public FileProvidedChecksumSource( FileProcessor fileProcessor )
+    {
+        this.fileProcessor = requireNonNull( fileProcessor );
+    }
+
+    @Override
+    public Map<String, String> getProvidedMetadataChecksums( RepositorySystemSession session,
+                                                             MetadataDownload transfer,
+                                                             RepositoryLayout repositoryLayout )
+    {
+        URI baseURI = getBaseURI( session );
+        if ( baseURI == null )
+        {
+            return null;
+        }
+
+        HashMap<String, String> checksums = new HashMap<>();
+        Metadata metadata = transfer.getMetadata();
+        List<ChecksumLocation> checksumLocations = repositoryLayout.getChecksumLocations(
+            metadata, false, repositoryLayout.getLocation( metadata, false )
+        );
+        for ( ChecksumLocation checksumLocation : checksumLocations )
+        {
+            Path checksumPath = new File( baseURI.resolve( checksumLocation.getLocation() ) ).toPath();
+            if ( Files.isReadable( checksumPath ) )
+            {
+                try
+                {
+                    String checksum = fileProcessor.readChecksum( checksumPath.toFile() );
+                    if ( checksum != null )
+                    {
+                        if ( LOGGER.isDebugEnabled() )
+                        {
+                            LOGGER.debug( "{} -> {}:{}", metadata,
+                                checksumLocation.getChecksumAlgorithmFactory().getName(), checksum );
+                        }
+
+                        checksums.put( checksumLocation.getChecksumAlgorithmFactory().getName(), checksum );
+                    }
+                }
+                catch ( IOException e )
+                {
+                    LOGGER.warn( "Could not read provided checksum for {} at path {}", metadata, checksumPath, e );
+                }
+            }
+        }
+        return checksums.isEmpty() ? null : checksums;
+    }
+
+    @Override
+    public Map<String, String> getProvidedArtifactChecksums( RepositorySystemSession session,
+                                                             ArtifactDownload transfer,
+                                                             RepositoryLayout repositoryLayout )
+    {
+        URI baseURI = getBaseURI( session );
+        if ( baseURI == null )
+        {
+            return null;
+        }
+
+        HashMap<String, String> checksums = new HashMap<>();
+        Artifact artifact = transfer.getArtifact();
+        List<ChecksumLocation> checksumLocations = repositoryLayout.getChecksumLocations(
+            artifact, false, repositoryLayout.getLocation( artifact, false )
+        );
+        for ( ChecksumLocation checksumLocation : checksumLocations )
+        {
+            Path checksumPath = new File( baseURI.resolve( checksumLocation.getLocation() ) ).toPath();
+            if ( Files.isReadable( checksumPath ) )
+            {
+                try
+                {
+                    String checksum = fileProcessor.readChecksum( checksumPath.toFile() );
+                    if ( checksum != null )
+                    {
+                        if ( LOGGER.isDebugEnabled() )
+                        {
+                            LOGGER.debug( "{} -> {}:{}", ArtifactIdUtils.toId( artifact ),

Review comment:
       Why does this one use `toId()` and the other one not?

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/AbstractChecksumPolicy.java
##########
@@ -41,26 +41,26 @@ protected AbstractChecksumPolicy( TransferResource resource )
     }
 
     @Override
-    public boolean onChecksumMatch( String algorithm, int kind )
+    public boolean onChecksumMatch( String algorithm, ChecksumKind kind )
     {
         requireNonNull( algorithm, "algorithm cannot be null" );
         return true;
     }
 
     @Override
-    public void onChecksumMismatch( String algorithm, int kind, ChecksumFailureException exception )
+    public void onChecksumMismatch( String algorithm, ChecksumKind kind, ChecksumFailureException exception )
             throws ChecksumFailureException
     {
         requireNonNull( algorithm, "algorithm cannot be null" );
         requireNonNull( exception, "exception cannot be null" );
-        if ( ( kind & KIND_UNOFFICIAL ) == 0 )
+        if ( ChecksumKind.REMOTE == kind )

Review comment:
       Why is `UNOFFICIAL` now remote?

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/impl/guice/AetherModule.java
##########
@@ -207,6 +212,17 @@ protected void configure()
 
     }
 
+    @Provides
+    @Singleton
+    Map<String, ProvidedChecksumsSource> provideDownloadChecksumSources(

Review comment:
       Stupid question: What is the purpose of `Download`? Is there going to be `Upload` as well?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] cstamas commented on a change in pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#discussion_r795067235



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/FileProvidedChecksumSource.java
##########
@@ -0,0 +1,188 @@
+package org.eclipse.aether.internal.impl;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.metadata.Metadata;
+import org.eclipse.aether.spi.connector.ArtifactDownload;
+import org.eclipse.aether.spi.connector.MetadataDownload;
+import org.eclipse.aether.spi.connector.checksum.ProvidedChecksumsSource;
+import org.eclipse.aether.spi.connector.layout.RepositoryLayout;
+import org.eclipse.aether.spi.connector.layout.RepositoryLayout.ChecksumLocation;
+import org.eclipse.aether.spi.io.FileProcessor;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * {@link ProvidedChecksumsSource} that does use specified directory structure to look up checksums.
+ *
+ * @since TBD
+ */
+@Singleton
+@Named( FileProvidedChecksumSource.NAME )
+public final class FileProvidedChecksumSource
+    implements ProvidedChecksumsSource
+{
+    public static final String NAME = "file";
+
+    static final String CONFIG_PROP_BASE_DIR = "aether.artifactResolver.providedChecksumSource.file.baseDir";
+
+    static final String LOCAL_REPO_PREFIX = ".checksums";
+
+    private static final Logger LOGGER = LoggerFactory.getLogger( FileProvidedChecksumSource.class );
+
+    private final FileProcessor fileProcessor;
+
+    @Inject
+    public FileProvidedChecksumSource( FileProcessor fileProcessor )
+    {
+        this.fileProcessor = requireNonNull( fileProcessor );
+    }
+
+    @Override
+    public Map<String, String> getProvidedMetadataChecksums( RepositorySystemSession session,

Review comment:
       Collapsed into private method




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] cstamas commented on a change in pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
cstamas commented on a change in pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#discussion_r795065476



##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/AbstractChecksumPolicy.java
##########
@@ -41,26 +41,26 @@ protected AbstractChecksumPolicy( TransferResource resource )
     }
 
     @Override
-    public boolean onChecksumMatch( String algorithm, int kind )
+    public boolean onChecksumMatch( String algorithm, ChecksumKind kind )
     {
         requireNonNull( algorithm, "algorithm cannot be null" );
         return true;
     }
 
     @Override
-    public void onChecksumMismatch( String algorithm, int kind, ChecksumFailureException exception )
+    public void onChecksumMismatch( String algorithm, ChecksumKind kind, ChecksumFailureException exception )
             throws ChecksumFailureException
     {
         requireNonNull( algorithm, "algorithm cannot be null" );
         requireNonNull( exception, "exception cannot be null" );
-        if ( ( kind & KIND_UNOFFICIAL ) == 0 )
+        if ( ChecksumKind.REMOTE == kind )

Review comment:
       `( kind & KIND_UNOFFICIAL ) == 0` is official, no (result is zero). "Official" now is explicit.

##########
File path: maven-resolver-impl/src/main/java/org/eclipse/aether/impl/guice/AetherModule.java
##########
@@ -207,6 +212,17 @@ protected void configure()
 
     }
 
+    @Provides
+    @Singleton
+    Map<String, ProvidedChecksumsSource> provideDownloadChecksumSources(

Review comment:
       fixed




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-resolver] michael-o commented on pull request #141: [MRESOLVER-234] Provided checksums

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #141:
URL: https://github.com/apache/maven-resolver/pull/141#issuecomment-1025221029


   Faked checksum, does not fail for me:
   ```
    88 [DEBUG] org.springframework.boot:spring-boot-starter-parent:pom:2.6.3 -> SHA-1:aaaaaaaabbbbffffff
    89 [INFO] Downloading from central: https://repo.maven.apache.org/maven2/org/springframework/boot/spring-boot-starter-parent/2.6.3/spring-boot-s
    90 [INFO] Downloaded from central: https://repo.maven.apache.org/maven2/org/springframework/boot/spring-boot-starter-parent/2.6.3/spring-boot-st
    91 [DEBUG] Writing tracking file /net/home/osipovmi/.m2/repository/org/springframework/boot/spring-boot-starter-parent/2.6.3/_remote.repositorie
    92 [DEBUG] Writing tracking file /net/home/osipovmi/.m2/repository/org/springframework/boot/spring-boot-starter-parent/2.6.3/spring-boot-starter
    93 [DEBUG] Extension realms for project com.java:build-with-deeply-structure-sentences-may-prohibilitively-slow:jar:1.0-SNAPSHOT: (none)
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org