-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[MNG-7344] track dependencyManagement import location in effective Model for MPH-183 #603
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 10 commits
2767730
550bb4d
198c581
38dc034
069ae93
3cd4d9d
b50cda2
9881c22
941b502
71c2989
21541ed
1a9aa52
237de75
73f5ff2
b27c457
78b912c
1918fdc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,4 +20,6 @@ | |
|
|
||
| public interface InputLocationTracker { | ||
| InputLocation getLocation(Object field); | ||
|
|
||
| InputLocation getImportedFrom(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,17 +33,27 @@ public class InputSource implements Serializable { | |
| private final String modelId; | ||
| private final String location; | ||
| private final List<InputSource> inputs; | ||
| private final InputLocation importedFrom; | ||
|
|
||
| public InputSource(String modelId, String location) { | ||
| this.modelId = modelId; | ||
| this.location = location; | ||
| this.inputs = null; | ||
| this.importedFrom = null; | ||
| } | ||
|
|
||
| private InputSource(String modelId, String location, InputLocation importedFrom) { | ||
| this.modelId = modelId; | ||
| this.location = location; | ||
| this.inputs = null; | ||
| this.importedFrom = importedFrom; | ||
| } | ||
|
|
||
| public InputSource(Collection<InputSource> inputs) { | ||
| this.modelId = null; | ||
| this.location = null; | ||
| this.inputs = ImmutableCollections.copy(inputs); | ||
| this.importedFrom = null; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -64,6 +74,14 @@ public String getModelId() { | |
| return this.modelId; | ||
| } | ||
|
|
||
| public InputLocation getImportedFrom() { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
| return importedFrom; | ||
| } | ||
|
|
||
| public InputSource importedFrom(InputLocation importedFrom) { | ||
| return new InputSource(modelId, location, importedFrom); | ||
|
||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1671,7 +1671,8 @@ private void importDependencyManagement( | |
|
|
||
| importIds.add(importing); | ||
|
|
||
| List<org.apache.maven.api.model.DependencyManagement> importMgmts = null; | ||
| // Model v4 | ||
| List<org.apache.maven.api.model.DependencyManagement> importMgmts = new ArrayList<>(); | ||
|
|
||
| for (Iterator<Dependency> it = depMgmt.getDependencies().iterator(); it.hasNext(); ) { | ||
| Dependency dependency = it.next(); | ||
|
|
@@ -1683,13 +1684,20 @@ private void importDependencyManagement( | |
|
|
||
| it.remove(); | ||
|
|
||
| // Model v3 | ||
| DependencyManagement importMgmt = loadDependencyManagement(model, request, problems, dependency, importIds); | ||
| if (importMgmt == null) { | ||
| continue; | ||
| } | ||
|
|
||
| if (importMgmt != null) { | ||
| if (importMgmts == null) { | ||
| importMgmts = new ArrayList<>(); | ||
| } | ||
|
|
||
| if (request.isLocationTracking()) { | ||
| // Keep track of why this DependencyManagement was imported. | ||
| // And map model v3 to model v4 -> importMgmt(v3).getDelegate() returns a v4 object | ||
| importMgmts.add( | ||
| org.apache.maven.api.model.DependencyManagement.newBuilder(importMgmt.getDelegate(), true) | ||
| .importedFrom(dependency.getDelegate().getLocation("")) | ||
|
||
| .build()); | ||
| } else { | ||
| importMgmts.add(importMgmt.getDelegate()); | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| /* | ||
| * 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. | ||
| */ | ||
| package org.apache.maven.model.composition; | ||
|
|
||
| import org.apache.maven.api.model.Dependency; | ||
| import org.apache.maven.api.model.DependencyManagement; | ||
| import org.apache.maven.api.model.InputLocation; | ||
| import org.apache.maven.api.model.InputSource; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| class DefaultDependencyManagementImporterTest { | ||
| @Test | ||
| void testUpdateWithImportedFrom_dependencyLocationAndBomLocationAreNull_dependencyReturned() { | ||
| final Dependency dependency = Dependency.newBuilder().build(); | ||
| final DependencyManagement depMgmt = DependencyManagement.newBuilder().build(); | ||
| final Dependency result = DefaultDependencyManagementImporter.updateWithImportedFrom(dependency, depMgmt); | ||
|
|
||
| assertThat(dependency).isEqualTo(result); | ||
| } | ||
|
|
||
| @Test | ||
| void testUpdateWithImportedFrom_dependencyManagementAndDependencyHaveSameSource_dependencyImportedFromSameSource() { | ||
| final InputSource source = new InputSource("SINGLE_SOURCE", ""); | ||
| final Dependency dependency = Dependency.newBuilder() | ||
| .location("", new InputLocation(1, 1, source)) | ||
| .build(); | ||
| final DependencyManagement bom = DependencyManagement.newBuilder() | ||
| .location("", new InputLocation(1, 1, source)) | ||
| .build(); | ||
|
|
||
| final Dependency result = DefaultDependencyManagementImporter.updateWithImportedFrom(dependency, bom); | ||
|
|
||
| assertThat(result).isNotNull(); | ||
| assertThat(result.getImportedFrom().toString()) | ||
| .isEqualTo(bom.getLocation("").toString()); | ||
| } | ||
|
|
||
| @Test | ||
| public void testUpdateWithImportedFrom_singleLevel_importedFromSet() { | ||
| // Arrange | ||
| final InputSource dependencySource = new InputSource("DEPENDENCY", "DEPENDENCY"); | ||
| final InputSource bomSource = new InputSource("BOM", "BOM"); | ||
| final Dependency dependency = Dependency.newBuilder() | ||
| .location("", new InputLocation(1, 1, dependencySource)) | ||
| .build(); | ||
| final DependencyManagement bom = DependencyManagement.newBuilder() | ||
| .location("", new InputLocation(2, 2, bomSource)) | ||
| .build(); | ||
|
|
||
| // Act | ||
| final Dependency result = DefaultDependencyManagementImporter.updateWithImportedFrom(dependency, bom); | ||
|
|
||
| // Assert | ||
| assertThat(result).isNotNull(); | ||
| assertThat(result.getImportedFrom().toString()) | ||
| .isEqualTo(bom.getLocation("").toString()); | ||
| } | ||
|
|
||
| @Test | ||
| public void testUpdateWithImportedFrom_multiLevel_importedFromSetChanged() { | ||
| // Arrange | ||
| final InputSource bomSource = new InputSource("BOM", "BOM"); | ||
| final InputSource intermediateSource = | ||
| new InputSource("INTERMEDIATE", "INTERMEDIATE").importedFrom(new InputLocation(bomSource)); | ||
| final InputSource dependencySource = | ||
| new InputSource("DEPENDENCY", "DEPENDENCY").importedFrom(new InputLocation(intermediateSource)); | ||
| final InputLocation bomLocation = new InputLocation(2, 2, bomSource); | ||
| final Dependency dependency = Dependency.newBuilder() | ||
| .location("", new InputLocation(1, 1, dependencySource)) | ||
| .importedFrom(bomLocation) | ||
| .build(); | ||
| final DependencyManagement bom = | ||
| DependencyManagement.newBuilder().location("", bomLocation).build(); | ||
|
|
||
| // Act | ||
| final Dependency result = DefaultDependencyManagementImporter.updateWithImportedFrom(dependency, bom); | ||
|
|
||
| // Assert | ||
| assertThat(result.getImportedFrom().toString()) | ||
| .isEqualTo(bom.getLocation("").toString()); | ||
| } | ||
|
|
||
| @Test | ||
| public void testUpdateWithImportedFrom_multiLevelAlreadyFoundInDifferentSource_importedFromSetMaintained() { | ||
| // Arrange | ||
| final InputSource bomSource = new InputSource("BOM", "BOM"); | ||
| final InputSource intermediateSource = | ||
| new InputSource("INTERMEDIATE", "INTERMEDIATE").importedFrom(new InputLocation(bomSource)); | ||
| final InputSource dependencySource = | ||
| new InputSource("DEPENDENCY", "DEPENDENCY").importedFrom(new InputLocation(intermediateSource)); | ||
| final Dependency dependency = Dependency.newBuilder() | ||
| .location("", new InputLocation(1, 1, dependencySource)) | ||
| .build(); | ||
| final DependencyManagement differentSource = DependencyManagement.newBuilder() | ||
| .location("", new InputLocation(2, 2, new InputSource("BOM2", "BOM2"))) | ||
| .build(); | ||
|
|
||
| // Act | ||
| final Dependency result = | ||
| DefaultDependencyManagementImporter.updateWithImportedFrom(dependency, differentSource); | ||
|
|
||
| // Assert | ||
| assertThat(result.getImportedFrom().toString()) | ||
| .isEqualTo(differentSource.getLocation("").toString()); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,11 @@ public final class InputLocation implements java.io.Serializable, Cloneable, Inp | |
| */ | ||
| private InputLocation location; | ||
|
|
||
| /** | ||
| * Field importedFrom. | ||
| */ | ||
| private InputLocation importedFrom; | ||
|
|
||
| // ----------------/ | ||
| // - Constructors -/ | ||
| // ----------------/ | ||
|
|
@@ -73,6 +78,7 @@ public InputLocation(org.apache.maven.api.model.InputLocation location) { | |
| .collect(Collectors.toMap( | ||
| e -> e.getKey(), | ||
| e -> e.getValue() == location ? this : new InputLocation(e.getValue()))); | ||
| this.importedFrom = location.getImportedFrom() != null ? new InputLocation(location.getImportedFrom()) : null; | ||
| } | ||
|
|
||
| public InputLocation(int lineNumber, int columnNumber) { | ||
|
|
@@ -217,6 +223,24 @@ public InputSource getSource() { | |
| return this.source; | ||
| } // -- InputSource getSource() | ||
|
|
||
| /** | ||
| * Get the imported from location. | ||
| * | ||
| * @return InputLocation | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
| */ | ||
| public InputLocation getImportedFrom() { | ||
| return importedFrom; | ||
| } | ||
|
|
||
| /** | ||
| * Set the imported from location. | ||
| * | ||
| * @param importedFrom | ||
| */ | ||
| public void setImportedFrom(InputLocation importedFrom) { | ||
| this.importedFrom = importedFrom; | ||
| } | ||
|
|
||
| /** | ||
| * Method merge. | ||
| * | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need an
@sincehere to rememberThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"that caused this model to be read."?
I need to dig more into code to propose another description, but this one is not clear