[MRESOLVER-294] Fix JapiCmp configuration and document it (#221)

Fix JapiCmp and describe what we promise and what we expect

And fix: no need for default interface method on an
interface that is noimplement. Once fixed, the override
can be removed from japicmp config as well.

---

https://issues.apache.org/jira/browse/MRESOLVER-294

maven-resolver-api/src/main/java/org/eclipse/aether/collection/CollectStepData.java

@@ -29,6 +29,8 @@
  *
  * @see org.eclipse.aether.RequestTrace
  * @since 1.8.1
+ * @noimplement This interface is not intended to be implemented by clients.
+ * @noextend This interface is not intended to be extended by clients.
  */
 public interface CollectStepData
 {

maven-resolver-connector-basic/src/test/java/org/eclipse/aether/connector/basic/TestChecksumAlgorithmSelector.java

@@ -22,7 +22,9 @@
 import java.nio.ByteBuffer;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
+import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 
@@ -32,6 +34,8 @@
 import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactorySupport;
 import org.eclipse.aether.util.ChecksumUtils;
 
+import static java.util.stream.Collectors.toList;
+
 /**
  * Test implementation of {@link ChecksumAlgorithmFactorySelector}.
  */
@@ -86,6 +90,14 @@ public String checksum()
         return new MessageDigestChecksumAlgorithmFactory( algorithm );
     }
 
+    @Override
+    public List<ChecksumAlgorithmFactory> selectList( Collection<String> algorithmNames )
+    {
+        return algorithmNames.stream()
+                .map( this::select )
+                .collect( toList() );
+    }
+
     private static class MessageDigestChecksumAlgorithmFactory
             extends ChecksumAlgorithmFactorySupport
     {

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/DefaultChecksumAlgorithmFactorySelector.java

@@ -24,6 +24,7 @@
 import javax.inject.Singleton;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -84,6 +85,14 @@ public ChecksumAlgorithmFactory select( String algorithmName )
         return factory;
     }
 
+    @Override
+    public List<ChecksumAlgorithmFactory> selectList( Collection<String> algorithmNames )
+    {
+        return algorithmNames.stream()
+                .map( this::select )
+                .collect( toList() );
+    }
+
     @Override
     public List<ChecksumAlgorithmFactory> getChecksumAlgorithmFactories()
     {

maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/resolution/TrustedChecksumsArtifactResolverPostProcessorTest.java

@@ -44,6 +44,7 @@
 import org.junit.Before;
 import org.junit.Test;
 
+import static java.util.stream.Collectors.toList;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.empty;
@@ -94,6 +95,14 @@ public ChecksumAlgorithmFactory select( String algorithmName )
                 throw new IllegalArgumentException("no alg factory for " + algorithmName);
             }
 
+            @Override
+            public List<ChecksumAlgorithmFactory> selectList( Collection<String> algorithmNames )
+            {
+                return algorithmNames.stream()
+                        .map( this::select )
+                        .collect( toList() );
+            }
+
             @Override
             public Collection<ChecksumAlgorithmFactory> getChecksumAlgorithmFactories()
             {

maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/checksum/ChecksumAlgorithmFactorySelector.java

@@ -22,8 +22,6 @@
 import java.util.Collection;
 import java.util.List;
 
-import static java.util.stream.Collectors.toList;
-
 /**
  * Component performing selection of {@link ChecksumAlgorithmFactory} based on known factory names.
  * Note: this component is NOT meant to be implemented or extended by client, is exposed ONLY to make clients
@@ -51,12 +49,7 @@ public interface ChecksumAlgorithmFactorySelector
      * @throws NullPointerException if passed in list of names is {@code null}.
      * @since 1.9.0
      */
-    default List<ChecksumAlgorithmFactory> selectList( Collection<String> algorithmNames )
-    {
-        return algorithmNames.stream()
-                .map( this::select )
-                .collect( toList() );
-    }
+    List<ChecksumAlgorithmFactory> selectList( Collection<String> algorithmNames );
 
     /**
      * Returns a collection of supported algorithms. This set represents ALL the algorithms supported by Resolver,

pom.xml

@@ -312,24 +312,54 @@
                 <type>${project.packaging}</type>
               </dependency>
             </oldVersion>
-            <parameter>
-              <onlyBinaryIncompatible>true</onlyBinaryIncompatible>
-              <breakBuildOnBinaryIncompatibleModifications>true</breakBuildOnBinaryIncompatibleModifications>
-              <overrideCompatibilityChangeParameters>
-                <overrideCompatibilityChangeParameter>
-                  <compatibilityChange>METHOD_NEW_DEFAULT</compatibilityChange>
-                  <binaryCompatible>true</binaryCompatible>
-                  <sourceCompatible>false</sourceCompatible>
-                </overrideCompatibilityChangeParameter>
-              </overrideCompatibilityChangeParameters>
-            </parameter>
           </configuration>
           <executions>
             <execution>
+              <id>default-source-cmp</id>
               <phase>verify</phase>
               <goals>
                 <goal>cmp</goal>
               </goals>
+              <configuration>
+                <parameter>
+                  <excludes>
+                    <exclude>org.eclipse.aether.RepositoryListener</exclude>
+                    <exclude>org.eclipse.aether.RepositorySystem</exclude>
+                    <exclude>org.eclipse.aether.RepositorySystemSession</exclude>
+                    <exclude>org.eclipse.aether.SessionData</exclude>
+                    <exclude>org.eclipse.aether.artifact.Artifact</exclude>
+                    <exclude>org.eclipse.aether.artifact.ArtifactType</exclude>
+                    <exclude>org.eclipse.aether.collection.CollectStepData</exclude>
+                    <exclude>org.eclipse.aether.collection.DependencyCollectionContext</exclude>
+                    <exclude>org.eclipse.aether.collection.DependencyGraphTransformationContext</exclude>
+                    <exclude>org.eclipse.aether.collection.VersionFilter.VersionFilterContext</exclude>
+                    <exclude>org.eclipse.aether.graph.DependencyCycle</exclude>
+                    <exclude>org.eclipse.aether.graph.DependencyNode</exclude>
+                    <exclude>org.eclipse.aether.metadata.Metadata</exclude>
+                    <exclude>org.eclipse.aether.repository.ArtifactRepository</exclude>
+                    <exclude>org.eclipse.aether.transfer.TransferListener</exclude>
+                    <exclude>org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactorySelector</exclude>
+                    <exclude>org.eclipse.aether.spi.connector.checksum.ChecksumPolicyProvider</exclude>
+                    <exclude>org.eclipse.aether.spi.connector.layout.RepositoryLayoutProvider</exclude>
+                    <exclude>org.eclipse.aether.spi.connector.transport.TransporterProvider</exclude>
+                  </excludes>
+                  <breakBuildOnBinaryIncompatibleModifications>false</breakBuildOnBinaryIncompatibleModifications>
+                  <breakBuildOnSourceIncompatibleModifications>true</breakBuildOnSourceIncompatibleModifications>
+                </parameter>
+              </configuration>
+            </execution>
+            <execution>
+              <id>default-binary-cmp</id>
+              <phase>verify</phase>
+              <goals>
+                <goal>cmp</goal>
+              </goals>
+              <configuration>
+                <parameter>
+                  <breakBuildOnBinaryIncompatibleModifications>true</breakBuildOnBinaryIncompatibleModifications>
+                  <breakBuildOnSourceIncompatibleModifications>false</breakBuildOnSourceIncompatibleModifications>
+                </parameter>
+              </configuration>
             </execution>
           </executions>
         </plugin>

src/site/markdown/api-compatibility.md

@@ -0,0 +1,113 @@
+# API Compatibility
+
+<!--
+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.
+-->
+
+Maven Resolver exposes three modules for clients and those extending Maven Resolver:
+* maven-resolver-api (in short API) -- for clients and those extending it
+* maven-resolver-spi (in short SPI) -- for those extending it
+* maven-resolver-util (in short Util) -- for client and those extending it
+
+Each module guarantees non-breaking (source and binary) compatibility, as long
+clients and extenders obey some rules. If you break any of these rules, you are
+prone to breakage, and you are on your own.
+
+## Interface And (Abstract) Class Level Contracts
+
+In source, we use two important Javadoc tags to mark intent:
+* `@noextend` -- classes (or interfaces) carrying this tag MUST NOT be extended
+* `@noimplement` -- interfaces carrying this tag MUST NOT be directly or indirectly implemented, 
+  UNLESS the Javadoc of given interface points to an abstract support class that makes indirect
+  implementation possible.
+
+Examples:
+
+* `RepositorySystem` interface. It carries both `@noextend` and `@noimplement` tags. This interface
+  MUST NOT be extended nor implemented. This is a component interface, that is usually injected into
+  client application.
+* `TransferListener` interface. It carries both `@noextend` and `@noimplement` tags, but Javadoc
+  points at `AbstractTransferListener` as extension point. Hence, clients are NOT allowed to extend
+  this interface, nor to directly implement it, but, if custom listener is needed, it is warmly
+  advised to extend the given abstract class. This way we can protect you from future breakage.
+
+## Package Level Contracts
+
+Maven Resolver implements customary habit to name packages NOT meant to be accessed by clients. 
+If a Java package contains following names:
+
+* `impl`
+* `internal`
+
+That Java package is meant as "internal" and does NOT offer guarantees of compatibility as API is. You
+may use classes from these packages, but again, you are on your own to deal with (binary or source)
+breakages. If you think a class from such package should be "pulled out" and made part of SPI or
+maybe API, better inform us via [JIRA](https://issues.apache.org/jira/projects/MRESOLVER): create a
+ticket and let's discuss.
+
+As a side note, the count of those names in Java package is directly proportional to possibility of 
+breaking changes: the more, the larger the possibility of breakage even in minor releases.
+
+## Version Level Contracts
+
+Maven Resolver does NOT use "semantic versioning", but still tries at best to reflect contained
+changes using version number. We use "major.minor.patch" versioning on resolver with following 
+semantics:
+
+* On major version change, one should NOT expect any backward compatibility.
+* On minor version change, we TRY to keep backward compatibility for those "exposed" 3 modules: 
+  API, SPI and Util. Still, there are examples when we failed to do so, usually driven by new 
+  features.
+* On minor version change, we ENSURE backward compatibility for those "exposed" 3 modules: API, 
+  SPI and Util.
+
+In any of three version changes above, in areas where we do not offer guarantees, everything
+can happen.
+
+## Outside of Maven
+
+Applications integrating Maven Resolver outside of Maven has really simple job: all they have to
+ensure is that API, SPI, Util and the rest of resolver (impl, basic-connector and transports)
+have all same versions, and they can rely on  these backward compatibility contracts as explained
+above.
+
+## Inside of Maven
+
+Historically, Maven 3.1 (as Maven 3.0 used resolver from different package) provided API, SPI 
+and Impl from its own embedded resolver, while Util, Connector, if some plugin or extension
+depended on those, was resolved. This caused that a plugin may work with different versions
+of API, SPI, Impl or Connector. Given Resolver had API "frozen" for too long time, this was essentially
+not a problem, but still weird.
+
+This changes in Maven 3.9+: Maven starting with version 3.9.0 will provide API, SPI, Impl 
+**and Util and Connector**. Reason for this change is that Impl and Connector bundled in Maven 
+implements things from both, API and SPI, and there was a binary incompatible change between 
+Resolver 1.8.0 and previous versions.
+
+Most Resolver users should not be affected by this change.
+
+The binary incompatible change happened in SPI class `RepositoryLayout` as part of work done for 
+[MRESOLVER-230](https://issues.apache.org/jira/browse/MRESOLVER-230), and affects both, Connector
+and Impl.
+
+## Backward Compatibility Checks
+
+To ensure backward compatibility, starting from 1.9.0 version Maven Resolver uses 
+[JApiCmp](https://siom79.github.io/japicmp/MavenPlugin.html),
+with two executions (for source and binary level checks). The plugin is enabled on 3 modules of
+Resolver mentioned at page top: API, SPI and Util. For "baseline" we use version 1.8.0.

src/site/site.xml

@@ -26,6 +26,7 @@ under the License.
   <body>
     <menu name="Overview">
       <item name="Introduction" href="index.html"/>
+      <item name="API Compatibility" href="api-compatibility.html"/>
       <item name="Configuration" href="configuration.html"/>
       <item name="About Checksums" href="about-checksums.html"/>
       <item name="About Local Repository" href="local-repository.html"/>