Repository: iluwatar/java-design-patterns
Issue: Dependency Conflict: duplicate classes "org.apache.commons.logging.LogFactory" in different JARs, have different implementations
Labels: epic: build issue

Hi, in **java-design-patterns-1.20.0** (**api-gateway\api-gateway-service** module), duplicate classes with the same fully-qualified name _**org.apache.commons.logging.LogFactory**_ are included in two different libraries, i.e., **org.slf4j:jcl-over-slf4j:1.7.16** and **commons-logging:commons-logging:1.2**.

According to _"first declaration wins"_ class loading strategy, only this class in **_org.slf4j:jcl-over-slf4j:1.7.16_** can be loaded, and that in _**commons-logging:commons-logging:1.2**_ will be shadowed.

By further analyzing, your project expects to invoke method **_<org.apache.commons.logging.LogFactory: org.apache.commons.logging.LogFactory getFactory()>_**  in _**commons-logging:commons-logging:1.2**_. As it has been shadowed, so that this method defined in **_org.slf4j:jcl-over-slf4j:1.7.16_** are actually forced to be referenced via the following invocation path:

```
<com.iluwatar.api.gateway.PriceClientImpl: java.lang.String getPrice()> D:\testcase\TestProject\java-design-patterns-1.20.0\api-gateway\api-gateway-service\target\classes
<org.apache.http.impl.client.HttpClients: org.apache.http.impl.client.CloseableHttpClient createDefault()> D:\cEnvironment\repository\org\apache\httpcomponents\httpclient\4.5.2\httpclient-4.5.2.jar
<org.apache.http.impl.client.HttpClientBuilder: org.apache.http.impl.client.CloseableHttpClient build()> D:\cEnvironment\repository\org\apache\httpcomponents\httpclient\4.5.2\httpclient-4.5.2.jar
<org.apache.http.impl.client.InternalHttpClient: void <init>(org.apache.http.impl.execchain.ClientExecChain,org.apache.http.conn.HttpClientConnectionManager,org.apache.http.conn.routing.HttpRoutePlanner,org.apache.http.config.Lookup,org.apache.http.config.Lookup,org.apache.http.client.CookieStore,org.apache.http.client.CredentialsProvider,org.apache.http.client.config.RequestConfig,java.util.List)> D:\cEnvironment\repository\org\apache\httpcomponents\httpclient\4.5.2\httpclient-4.5.2.jar
<org.apache.commons.logging.LogFactory: org.apache.commons.logging.Log getLog(java.lang.Class)> 
<org.apache.commons.logging.LogFactory: org.apache.commons.logging.LogFactory getFactory()>

```
Although both of these two conflicting classes contain the referenced methods (with the same signature), they have different implementations. This issue will not lead to runtime crashes, but it can introduce inconsistent semantic hehavior by changing the control flows and data flows.

**_Workaround solution:_**
An easy way to workaround the problem is **_reversing the declaration order_** of these two libraries (i.e., reverse the declaration order of httpclient and spring-boot-starter-web) in pom file. 
Then, according to **_"first declaration wins"_** class loading strategy, class _**org.apache.commons.logging.LogFactory**_ in _**commons-logging:commons-logging:1.2**_ can be loaded (the version that **java-design-patterns** expects to reference by static analysis).
This fix will not affect other libraries or class, except the above duplicate class.



### Dependency tree---
[INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ api-gateway-service ---
[INFO] com.iluwatar:api-gateway-service:jar:1.20.0
[INFO] +- org.springframework:spring-webmvc:jar:4.2.4.RELEASE:compile
[INFO] |  +- org.springframework:spring-beans:jar:4.2.5.RELEASE:compile
[INFO] |  +- org.springframework:spring-context:jar:4.2.5.RELEASE:compile
[INFO] |  |  \- org.springframework:spring-aop:jar:4.2.5.RELEASE:compile
[INFO] |  |     \- aopalliance:aopalliance:jar:1.0:compile
[INFO] |  +- org.springframework:spring-core:jar:4.2.5.RELEASE:compile
[INFO] |  +- org.springframework:spring-expression:jar:4.2.5.RELEASE:compile
[INFO] |  \- org.springframework:spring-web:jar:4.2.5.RELEASE:compile
[INFO] +- org.springframework.boot:spring-boot-starter-web:jar:1.3.3.RELEASE:compile
[INFO] |  +- org.springframework.boot:spring-boot-starter:jar:1.3.3.RELEASE:compile
[INFO] |  |  +- org.springframework.boot:spring-boot:jar:1.3.3.RELEASE:compile
[INFO] |  |  +- org.springframework.boot:spring-boot-autoconfigure:jar:1.3.3.RELEASE:compile
[INFO] |  |  +- org.springframework.boot:spring-boot-starter-logging:jar:1.3.3.RELEASE:compile
[INFO] |  |  |  +- _**org.slf4j:jcl-over-slf4j:jar:1.7.16:compile**_
[INFO] |  |  |  +- org.slf4j:jul-to-slf4j:jar:1.7.16:compile
[INFO] |  |  |  \- org.slf4j:log4j-over-slf4j:jar:1.7.16:compile
[INFO] |  |  \- org.yaml:snakeyaml:jar:1.16:runtime
[INFO] |  +- org.springframework.boot:spring-boot-starter-tomcat:jar:1.3.3.RELEASE:compile
[INFO] |  |  +- org.apache.tomcat.embed:tomcat-embed-core:jar:8.0.32:compile
[INFO] |  |  +- org.apache.tomcat.embed:tomcat-embed-el:jar:8.0.32:compile
[INFO] |  |  +- org.apache.tomcat.embed:tomcat-embed-logging-juli:jar:8.0.32:compile
[INFO] |  |  \- org.apache.tomcat.embed:tomcat-embed-websocket:jar:8.0.32:compile
[INFO] |  +- org.springframework.boot:spring-boot-starter-validation:jar:1.3.3.RELEASE:compile
[INFO] |  |  \- org.hibernate:hibernate-validator:jar:5.2.4.Final:compile
[INFO] |  |     +- javax.validation:validation-api:jar:1.1.0.Final:compile
[INFO] |  |     +- org.jboss.logging:jboss-logging:jar:3.3.0.Final:compile
[INFO] |  |     \- com.fasterxml:classmate:jar:1.1.0:compile
[INFO] |  \- com.fasterxml.jackson.core:jackson-databind:jar:2.6.5:compile
[INFO] |     +- com.fasterxml.jackson.core:jackson-annotations:jar:2.6.5:compile
[INFO] |     \- com.fasterxml.jackson.core:jackson-core:jar:2.6.5:compile
[INFO] +- org.junit.jupiter:junit-jupiter-api:jar:5.0.2:test
[INFO] |  +- org.opentest4j:opentest4j:jar:1.0.0:test
[INFO] |  \- org.junit.platform:junit-platform-commons:jar:1.0.2:test
[INFO] +- org.junit.jupiter:junit-jupiter-engine:jar:5.0.2:test
[INFO] |  \- org.junit.platform:junit-platform-engine:jar:1.0.2:test
[INFO] +- org.mockito:mockito-core:jar:1.10.19:test
[INFO] |  +- org.hamcrest:hamcrest-core:jar:1.3:test
[INFO] |  \- org.objenesis:objenesis:jar:2.1:test
[INFO] +- org.apache.httpcomponents:httpclient:jar:4.5.2:compile
[INFO] |  +- org.apache.httpcomponents:httpcore:jar:4.4.4:compile
[INFO] |  +- _**commons-logging:commons-logging:jar:1.2:compile**_
[INFO] |  \- commons-codec:commons-codec:jar:1.9:compile
[INFO] +- org.slf4j:slf4j-api:jar:1.7.21:compile
[INFO] +- ch.qos.logback:logback-classic:jar:1.1.7:compile
[INFO] \- ch.qos.logback:logback-core:jar:1.1.7:compile

Thank you very much.
Best,
Coco

Issue comments:
Code snippet of **_<org.apache.commons.logging.LogFactory: org.apache.commons.logging.LogFactory getFactory()>_** in _**org.slf4j:jcl-over-slf4j:jar:1.7.16:compile**_ (loaded version):
![01](https://user-images.githubusercontent.com/46306510/55683799-534bcb80-5976-11e9-800a-a3b00a00cafa.png)

Code snippet of **_<org.apache.commons.logging.LogFactory: org.apache.commons.logging.LogFactory getFactory()>_** in _**commons-logging:commons-logging:jar:1.2:compile**_ 
 (shadowed but expected to invoke method):
![02](https://user-images.githubusercontent.com/46306510/55683800-5777e900-5976-11e9-82f0-3e7a8e44bff2.png)
**.......**


As a result, these conflicting method included in **commons-logging:commons-logging:jar:1.2:compile** deals with different cases, which changes the control flows and data flows. So being forced to use these methods in **org.slf4j:jcl-over-slf4j:jar:1.7.16:compile** may lead to inconsisitent semantic behaviors.
Thanks @HelloCoCooo for the finding. I'll be glad to accept your pull request.

Pull request review comments:

Patch that resolved the issue:
diff --git a/api-gateway/api-gateway-service/pom.xml b/api-gateway/api-gateway-service/pom.xml
index f947049a757c..13bbddc97d09 100644
--- a/api-gateway/api-gateway-service/pom.xml
+++ b/api-gateway/api-gateway-service/pom.xml
@@ -48,6 +48,10 @@
       <groupId>org.springframework</groupId>
       <artifactId>spring-webmvc</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.apache.httpcomponents</groupId>
+      <artifactId>httpclient</artifactId>
+    </dependency>
     <dependency>
       <groupId>org.springframework.boot</groupId>
       <artifactId>spring-boot-starter-web</artifactId>
@@ -67,10 +71,6 @@
       <artifactId>mockito-core</artifactId>
       <scope>test</scope>
     </dependency>
-    <dependency>
-      <groupId>org.apache.httpcomponents</groupId>
-      <artifactId>httpclient</artifactId>
-    </dependency>
   </dependencies>
 
   <build>
@@ -89,4 +89,4 @@
       </plugin>
     </plugins>
   </build>
-</project>
\ No newline at end of file
+</project>


Test patch: