Pankraz76 commented on code in PR #2380:
URL: https://github.com/apache/maven/pull/2380#discussion_r2126898197
##########
impl/maven-core/src/main/java/org/apache/maven/internal/transformation/impl/PomInlinerTransformer.java:
##########
@@ -52,35 +61,79 @@ class PomInlinerTransformer extends TransformerSupport {
this.interpolator = requireNonNull(interpolator);
}
+ @Override
+ public InstallRequest remapInstallArtifacts(RepositorySystemSession
session, InstallRequest request) {
+ return request.setArtifacts(replacePom(session,
request.getArtifacts()));
+ }
+
+ @Override
+ public DeployRequest remapDeployArtifacts(RepositorySystemSession session,
DeployRequest request) {
+ return request.setArtifacts(replacePom(session,
request.getArtifacts()));
+ }
+
+ private Collection<Artifact> replacePom(RepositorySystemSession session,
Collection<Artifact> artifacts) {
+ Set<String> needsInlining = needsInlining(session);
+ if (needsInlining.isEmpty()) {
+ return artifacts;
+ }
+ ArrayList<Artifact> newArtifacts = new ArrayList<>(artifacts.size());
+ for (Artifact artifact : artifacts) {
+ if ("pom".equals(artifact.getExtension())
+ && artifact.getClassifier().isEmpty()) {
+ try {
+ Path tmpPom = Files.createTempFile("pom-inliner-", ".xml");
+ String originalPom = Files.readString(artifact.getPath());
+ String interpolatedPom = interpolator.interpolate(
+ originalPom,
+ property -> {
+ if (needsInlining.contains(property)) {
+ return (String)
+
session.getConfigProperties().get(property);
+ }
+ return null;
+ },
+ false);
+ if (!Objects.equals(originalPom, interpolatedPom)) {
+ Files.writeString(tmpPom, interpolatedPom);
+ artifact = artifact.setPath(tmpPom);
+ }
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ }
+ newArtifacts.add(artifact);
+ }
+ return newArtifacts;
+ }
+
+ @SuppressWarnings("unchecked")
+ private Set<String> needsInlining(RepositorySystemSession session) {
+ return (Set<String>) session.getData()
+ .computeIfAbsent(
+ PomInlinerTransformer.class.getName() +
".needsInlining", ConcurrentHashMap::newKeySet);
+ }
+
@Override
public void injectTransformedArtifacts(RepositorySystemSession session,
MavenProject project) throws IOException {
if (!Features.consumerPom(session.getConfigProperties())) {
try {
Model model = read(project.getFile().toPath());
- boolean parentVersion = false;
String version = model.getVersion();
if (version == null && model.getParent() != null) {
- parentVersion = true;
version = model.getParent().getVersion();
}
String newVersion;
if (version != null) {
+ HashSet<String> usedProperties = new HashSet<>();
newVersion = interpolator.interpolate(version.trim(),
property -> {
if
(!session.getConfigProperties().containsKey(property)) {
throw new IllegalArgumentException("Cannot inline
property " + property);
}
+ usedProperties.add(property);
return (String)
session.getConfigProperties().get(property);
});
if (!Objects.equals(version, newVersion)) {
- if (parentVersion) {
- model =
model.withParent(model.getParent().withVersion(newVersion));
- } else {
- model = model.withVersion(newVersion);
- }
- Path tmpPom = Files.createTempFile(
- project.getArtifactId() + "-" +
project.getVersion() + "-", ".xml");
- write(model, tmpPom);
- project.setFile(tmpPom.toFile());
+ needsInlining(session).addAll(usedProperties);
Review Comment:
yes.
##########
impl/maven-core/src/main/java/org/apache/maven/internal/transformation/impl/PomInlinerTransformer.java:
##########
@@ -52,35 +61,79 @@ class PomInlinerTransformer extends TransformerSupport {
this.interpolator = requireNonNull(interpolator);
}
+ @Override
+ public InstallRequest remapInstallArtifacts(RepositorySystemSession
session, InstallRequest request) {
+ return request.setArtifacts(replacePom(session,
request.getArtifacts()));
+ }
+
+ @Override
+ public DeployRequest remapDeployArtifacts(RepositorySystemSession session,
DeployRequest request) {
+ return request.setArtifacts(replacePom(session,
request.getArtifacts()));
+ }
+
+ private Collection<Artifact> replacePom(RepositorySystemSession session,
Collection<Artifact> artifacts) {
+ Set<String> needsInlining = needsInlining(session);
+ if (needsInlining.isEmpty()) {
+ return artifacts;
+ }
+ ArrayList<Artifact> newArtifacts = new ArrayList<>(artifacts.size());
+ for (Artifact artifact : artifacts) {
+ if ("pom".equals(artifact.getExtension())
+ && artifact.getClassifier().isEmpty()) {
+ try {
+ Path tmpPom = Files.createTempFile("pom-inliner-", ".xml");
+ String originalPom = Files.readString(artifact.getPath());
+ String interpolatedPom = interpolator.interpolate(
+ originalPom,
+ property -> {
+ if (needsInlining.contains(property)) {
+ return (String)
+
session.getConfigProperties().get(property);
+ }
+ return null;
+ },
+ false);
+ if (!Objects.equals(originalPom, interpolatedPom)) {
+ Files.writeString(tmpPom, interpolatedPom);
+ artifact = artifact.setPath(tmpPom);
+ }
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ }
+ newArtifacts.add(artifact);
+ }
+ return newArtifacts;
+ }
+
+ @SuppressWarnings("unchecked")
+ private Set<String> needsInlining(RepositorySystemSession session) {
+ return (Set<String>) session.getData()
+ .computeIfAbsent(
+ PomInlinerTransformer.class.getName() +
".needsInlining", ConcurrentHashMap::newKeySet);
+ }
+
@Override
public void injectTransformedArtifacts(RepositorySystemSession session,
MavenProject project) throws IOException {
if (!Features.consumerPom(session.getConfigProperties())) {
try {
Model model = read(project.getFile().toPath());
- boolean parentVersion = false;
String version = model.getVersion();
if (version == null && model.getParent() != null) {
- parentVersion = true;
version = model.getParent().getVersion();
}
String newVersion;
if (version != null) {
+ HashSet<String> usedProperties = new HashSet<>();
newVersion = interpolator.interpolate(version.trim(),
property -> {
if
(!session.getConfigProperties().containsKey(property)) {
throw new IllegalArgumentException("Cannot inline
property " + property);
}
+ usedProperties.add(property);
return (String)
session.getConfigProperties().get(property);
});
if (!Objects.equals(version, newVersion)) {
- if (parentVersion) {
Review Comment:
cohesion. good giving dedicated concern.
##########
impl/maven-core/src/main/java/org/apache/maven/internal/transformation/impl/PomInlinerTransformer.java:
##########
@@ -52,35 +61,79 @@ class PomInlinerTransformer extends TransformerSupport {
this.interpolator = requireNonNull(interpolator);
}
+ @Override
+ public InstallRequest remapInstallArtifacts(RepositorySystemSession
session, InstallRequest request) {
+ return request.setArtifacts(replacePom(session,
request.getArtifacts()));
+ }
+
+ @Override
+ public DeployRequest remapDeployArtifacts(RepositorySystemSession session,
DeployRequest request) {
+ return request.setArtifacts(replacePom(session,
request.getArtifacts()));
+ }
+
+ private Collection<Artifact> replacePom(RepositorySystemSession session,
Collection<Artifact> artifacts) {
+ Set<String> needsInlining = needsInlining(session);
+ if (needsInlining.isEmpty()) {
+ return artifacts;
+ }
+ ArrayList<Artifact> newArtifacts = new ArrayList<>(artifacts.size());
+ for (Artifact artifact : artifacts) {
+ if ("pom".equals(artifact.getExtension())
+ && artifact.getClassifier().isEmpty()) {
+ try {
+ Path tmpPom = Files.createTempFile("pom-inliner-", ".xml");
+ String originalPom = Files.readString(artifact.getPath());
+ String interpolatedPom = interpolator.interpolate(
+ originalPom,
+ property -> {
+ if (needsInlining.contains(property)) {
+ return (String)
+
session.getConfigProperties().get(property);
+ }
+ return null;
+ },
+ false);
+ if (!Objects.equals(originalPom, interpolatedPom)) {
+ Files.writeString(tmpPom, interpolatedPom);
+ artifact = artifact.setPath(tmpPom);
+ }
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ }
+ newArtifacts.add(artifact);
+ }
+ return newArtifacts;
+ }
+
+ @SuppressWarnings("unchecked")
+ private Set<String> needsInlining(RepositorySystemSession session) {
+ return (Set<String>) session.getData()
+ .computeIfAbsent(
+ PomInlinerTransformer.class.getName() +
".needsInlining", ConcurrentHashMap::newKeySet);
+ }
+
@Override
public void injectTransformedArtifacts(RepositorySystemSession session,
MavenProject project) throws IOException {
if (!Features.consumerPom(session.getConfigProperties())) {
try {
Model model = read(project.getFile().toPath());
- boolean parentVersion = false;
String version = model.getVersion();
if (version == null && model.getParent() != null) {
- parentVersion = true;
version = model.getParent().getVersion();
}
String newVersion;
if (version != null) {
+ HashSet<String> usedProperties = new HashSet<>();
Review Comment:
couplin.
##########
impl/maven-core/src/main/java/org/apache/maven/internal/transformation/impl/PomInlinerTransformer.java:
##########
@@ -52,35 +61,79 @@ class PomInlinerTransformer extends TransformerSupport {
this.interpolator = requireNonNull(interpolator);
}
+ @Override
+ public InstallRequest remapInstallArtifacts(RepositorySystemSession
session, InstallRequest request) {
+ return request.setArtifacts(replacePom(session,
request.getArtifacts()));
+ }
+
+ @Override
+ public DeployRequest remapDeployArtifacts(RepositorySystemSession session,
DeployRequest request) {
+ return request.setArtifacts(replacePom(session,
request.getArtifacts()));
+ }
+
+ private Collection<Artifact> replacePom(RepositorySystemSession session,
Collection<Artifact> artifacts) {
+ Set<String> needsInlining = needsInlining(session);
+ if (needsInlining.isEmpty()) {
+ return artifacts;
+ }
+ ArrayList<Artifact> newArtifacts = new ArrayList<>(artifacts.size());
+ for (Artifact artifact : artifacts) {
+ if ("pom".equals(artifact.getExtension())
+ && artifact.getClassifier().isEmpty()) {
+ try {
+ Path tmpPom = Files.createTempFile("pom-inliner-", ".xml");
+ String originalPom = Files.readString(artifact.getPath());
+ String interpolatedPom = interpolator.interpolate(
+ originalPom,
+ property -> {
+ if (needsInlining.contains(property)) {
+ return (String)
+
session.getConfigProperties().get(property);
+ }
+ return null;
+ },
+ false);
+ if (!Objects.equals(originalPom, interpolatedPom)) {
+ Files.writeString(tmpPom, interpolatedPom);
+ artifact = artifact.setPath(tmpPom);
+ }
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ }
+ newArtifacts.add(artifact);
+ }
+ return newArtifacts;
+ }
+
+ @SuppressWarnings("unchecked")
+ private Set<String> needsInlining(RepositorySystemSession session) {
+ return (Set<String>) session.getData()
+ .computeIfAbsent(
+ PomInlinerTransformer.class.getName() +
".needsInlining", ConcurrentHashMap::newKeySet);
+ }
+
@Override
public void injectTransformedArtifacts(RepositorySystemSession session,
MavenProject project) throws IOException {
if (!Features.consumerPom(session.getConfigProperties())) {
try {
Model model = read(project.getFile().toPath());
- boolean parentVersion = false;
String version = model.getVersion();
if (version == null && model.getParent() != null) {
- parentVersion = true;
version = model.getParent().getVersion();
}
String newVersion;
if (version != null) {
+ HashSet<String> usedProperties = new HashSet<>();
newVersion = interpolator.interpolate(version.trim(),
property -> {
if
(!session.getConfigProperties().containsKey(property)) {
throw new IllegalArgumentException("Cannot inline
property " + property);
}
+ usedProperties.add(property);
return (String)
session.getConfigProperties().get(property);
});
if (!Objects.equals(version, newVersion)) {
- if (parentVersion) {
- model =
model.withParent(model.getParent().withVersion(newVersion));
- } else {
- model = model.withVersion(newVersion);
- }
- Path tmpPom = Files.createTempFile(
- project.getArtifactId() + "-" +
project.getVersion() + "-", ".xml");
- write(model, tmpPom);
- project.setFile(tmpPom.toFile());
+ needsInlining(session).addAll(usedProperties);
Review Comment:
might give dedication as well, if makes sense.
```suggestion
needsInlining(session).addAll(getUsedProperties(interpolator));
```
might be not performant as n+1 iteration.
because the code does something like simple `map` operation on kind
collection interpolator.interpolate(). Idk this structure so well im sorry.
Somehow can not reach the code, so my knowledge very short.
##########
impl/maven-core/src/main/java/org/apache/maven/internal/transformation/impl/PomInlinerTransformer.java:
##########
@@ -52,35 +61,79 @@ class PomInlinerTransformer extends TransformerSupport {
this.interpolator = requireNonNull(interpolator);
}
+ @Override
+ public InstallRequest remapInstallArtifacts(RepositorySystemSession
session, InstallRequest request) {
+ return request.setArtifacts(replacePom(session,
request.getArtifacts()));
+ }
+
+ @Override
+ public DeployRequest remapDeployArtifacts(RepositorySystemSession session,
DeployRequest request) {
+ return request.setArtifacts(replacePom(session,
request.getArtifacts()));
+ }
+
+ private Collection<Artifact> replacePom(RepositorySystemSession session,
Collection<Artifact> artifacts) {
+ Set<String> needsInlining = needsInlining(session);
+ if (needsInlining.isEmpty()) {
+ return artifacts;
+ }
+ ArrayList<Artifact> newArtifacts = new ArrayList<>(artifacts.size());
+ for (Artifact artifact : artifacts) {
Review Comment:
```suggestion
return artifacts.map(....)
```
give special attention avoiding overhead giving only important stuff which
is for body as `map` operation as well. Its already very nice functional and
very compact. Lets embrace here as well.
its just 1 LOC
`return isEmpty() ? artifacts : artifacts.map(toFoo()) `
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
