Skip to content

Commit a8833e7

Browse files
cstamascstamas
authored
GH-1609 Bad metadata naming (#1610)
It caused in certain cases that lock names for different metadata collapsed onto same name, and hence, congestion with possible timeouts on resource happened. In case of metadata, `Metadata#getType` was totally ignored, which turns out was wrong, as today we have more types than the simple `maven-metadata.xml`. Fix: We already had means to make sure "string is valid for path segment", but it was buried in `RepositoryIdHelper`. Pull it out, and make it reusable, as this is essentially very same use case. Also, this extra work really "punishes" only non maven metadata, and that is okay (right now we have only repository prefix and archetype catalog files in play). Fixes #1609
1 parent 0dd7824 commit a8833e7

File tree

8 files changed

+421
-80
lines changed

8 files changed

+421
-80
lines changed

‎maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapper.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.eclipse.aether.artifact.Artifact;
2727
import org.eclipse.aether.metadata.Metadata;
2828
import org.eclipse.aether.named.NamedLockKey;
29+
import org.eclipse.aether.util.PathUtils;
2930

3031
import static java.util.Objects.requireNonNull;
3132

@@ -103,6 +104,8 @@ private static String getArtifactName(Artifact artifact, String prefix, String s
103104
+ suffix;
104105
}
105106

107+
private static final String MAVEN_METADATA = "maven-metadata.xml";
108+
106109
private static String getMetadataName(Metadata metadata, String prefix, String separator, String suffix) {
107110
String name = prefix;
108111
if (!metadata.getGroupId().isEmpty()) {
@@ -113,6 +116,13 @@ private static String getMetadataName(Metadata metadata, String prefix, String s
113116
name += separator + metadata.getVersion();
114117
}
115118
}
119+
if (!MAVEN_METADATA.equals(metadata.getType())) {
120+
name += separator + PathUtils.stringToPathSegment(metadata.getType());
121+
}
122+
} else {
123+
if (!MAVEN_METADATA.equals(metadata.getType())) {
124+
name += PathUtils.stringToPathSegment(metadata.getType());
125+
}
116126
}
117127
return name + suffix;
118128
}

‎maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirHashingNameMapperTest.java‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.eclipse.aether.internal.impl.synccontext.named;
20+
21+
import java.io.File;
22+
import java.io.IOException;
23+
import java.util.Collection;
24+
import java.util.Iterator;
25+
26+
import org.eclipse.aether.artifact.DefaultArtifact;
27+
import org.eclipse.aether.metadata.DefaultMetadata;
28+
import org.eclipse.aether.metadata.Metadata;
29+
import org.eclipse.aether.named.NamedLockKey;
30+
import org.junit.jupiter.api.Test;
31+
32+
import static java.util.Collections.emptyList;
33+
import static java.util.Collections.singletonList;
34+
import static org.junit.jupiter.api.Assertions.assertEquals;
35+
import static org.junit.jupiter.api.Assertions.assertTrue;
36+
37+
public class BasedirHashingNameMapperTest extends NameMapperTestSupport {
38+
private final String PS = "/"; // we work with URIs now, not OS file paths
39+
40+
BasedirNameMapper mapper = new BasedirNameMapper(new HashingNameMapper(GAVNameMapper.gav()));
41+
42+
@Test
43+
void nullsAndEmptyInputs() {
44+
Collection<NamedLockKey> names;
45+
46+
names = mapper.nameLocks(session, null, null);
47+
assertTrue(names.isEmpty());
48+
49+
names = mapper.nameLocks(session, null, emptyList());
50+
assertTrue(names.isEmpty());
51+
52+
names = mapper.nameLocks(session, emptyList(), null);
53+
assertTrue(names.isEmpty());
54+
55+
names = mapper.nameLocks(session, emptyList(), emptyList());
56+
assertTrue(names.isEmpty());
57+
}
58+
59+
@Test
60+
void defaultLocksDir() {
61+
configProperties.put("aether.syncContext.named.hashing.depth", "0");
62+
configProperties.put("aether.syncContext.named.basedir.locksDir", null);
63+
DefaultArtifact artifact = new DefaultArtifact("group:artifact:1.0");
64+
Collection<NamedLockKey> names = mapper.nameLocks(session, singletonList(artifact), null);
65+
assertEquals(1, names.size());
66+
assertEquals(
67+
names.iterator().next().name(),
68+
basedir.toUri() + PS + ".locks" + PS + "46e98183d232f1e16f863025080c7f2b9797fd10");
69+
}
70+
71+
@Test
72+
void relativeLocksDir() {
73+
configProperties.put("aether.syncContext.named.hashing.depth", "0");
74+
configProperties.put("aether.syncContext.named.basedir.locksDir", "my/locks");
75+
DefaultArtifact artifact = new DefaultArtifact("group:artifact:1.0");
76+
Collection<NamedLockKey> names = mapper.nameLocks(session, singletonList(artifact), null);
77+
assertEquals(1, names.size());
78+
assertEquals(
79+
names.iterator().next().name(),
80+
basedir.toUri() + PS + "my" + PS + "locks" + PS + "46e98183d232f1e16f863025080c7f2b9797fd10");
81+
}
82+
83+
@Test
84+
void absoluteLocksDir() throws IOException {
85+
String absoluteLocksDir = "/my/locks";
86+
String customBaseDir =
87+
new File(absoluteLocksDir).getCanonicalFile().toPath().toUri().toASCIIString();
88+
89+
configProperties.put("aether.syncContext.named.hashing.depth", "0");
90+
configProperties.put("aether.syncContext.named.basedir.locksDir", absoluteLocksDir);
91+
DefaultArtifact artifact = new DefaultArtifact("group:artifact:1.0");
92+
Collection<NamedLockKey> names = mapper.nameLocks(session, singletonList(artifact), null);
93+
assertEquals(1, names.size());
94+
assertEquals(names.iterator().next().name(), customBaseDir + PS + "46e98183d232f1e16f863025080c7f2b9797fd10");
95+
}
96+
97+
@Test
98+
void singleArtifact() {
99+
configProperties.put("aether.syncContext.named.hashing.depth", "0");
100+
101+
DefaultArtifact artifact = new DefaultArtifact("group:artifact:1.0");
102+
Collection<NamedLockKey> names = mapper.nameLocks(session, singletonList(artifact), null);
103+
assertEquals(1, names.size());
104+
assertEquals(
105+
names.iterator().next().name(),
106+
basedir.toUri() + PS + ".locks" + PS + "46e98183d232f1e16f863025080c7f2b9797fd10");
107+
}
108+
109+
@Test
110+
void singleMetadata() {
111+
configProperties.put("aether.syncContext.named.hashing.depth", "0");
112+
113+
DefaultMetadata metadata =
114+
new DefaultMetadata("group", "artifact", "maven-metadata.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT);
115+
Collection<NamedLockKey> names = mapper.nameLocks(session, null, singletonList(metadata));
116+
assertEquals(1, names.size());
117+
assertEquals(
118+
names.iterator().next().name(),
119+
basedir.toUri() + PS + ".locks" + PS + "293b3990971f4b4b02b220620d2538eaac5f221b");
120+
}
121+
122+
@Test
123+
void prefixMetadata() {
124+
configProperties.put("aether.syncContext.named.hashing.depth", "0");
125+
126+
DefaultMetadata metadata =
127+
new DefaultMetadata("", "", ".meta/prefixes-central.txt", Metadata.Nature.RELEASE_OR_SNAPSHOT);
128+
Collection<NamedLockKey> names = mapper.nameLocks(session, null, singletonList(metadata));
129+
assertEquals(1, names.size());
130+
assertEquals(
131+
names.iterator().next().name(),
132+
basedir.toUri() + PS + ".locks" + PS + "520e2ba3a365db8cd804bcc40df38e1a52987e0f");
133+
}
134+
135+
@Test
136+
void rootSomeMetadata() {
137+
configProperties.put("aether.syncContext.named.hashing.depth", "0");
138+
139+
DefaultMetadata metadata = new DefaultMetadata("", "", "something.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT);
140+
Collection<NamedLockKey> names = mapper.nameLocks(session, null, singletonList(metadata));
141+
assertEquals(1, names.size());
142+
assertEquals(
143+
names.iterator().next().name(),
144+
basedir.toUri() + PS + ".locks" + PS + "e75ca04110613537eeb805ac3cc3a3bb4b3b999a");
145+
}
146+
147+
@Test
148+
void nonRootSomeMetadata() {
149+
configProperties.put("aether.syncContext.named.hashing.depth", "0");
150+
151+
DefaultMetadata metadata =
152+
new DefaultMetadata("groupId", "artifactId", "something.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT);
153+
Collection<NamedLockKey> names = mapper.nameLocks(session, null, singletonList(metadata));
154+
assertEquals(1, names.size());
155+
assertEquals(
156+
names.iterator().next().name(),
157+
basedir.toUri() + PS + ".locks" + PS + "3ebd7051578a145a78fc67adeaccdcdc5a914b28");
158+
}
159+
160+
@Test
161+
void oneAndOne() {
162+
configProperties.put("aether.syncContext.named.hashing.depth", "0");
163+
164+
DefaultArtifact artifact = new DefaultArtifact("agroup:artifact:1.0");
165+
DefaultMetadata metadata =
166+
new DefaultMetadata("bgroup", "artifact", "maven-metadata.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT);
167+
Collection<NamedLockKey> names = mapper.nameLocks(session, singletonList(artifact), singletonList(metadata));
168+
169+
assertEquals(names.size(), 2);
170+
Iterator<NamedLockKey> namesIterator = names.iterator();
171+
172+
// they are sorted as well
173+
assertEquals(
174+
namesIterator.next().name(),
175+
basedir.toUri() + PS + ".locks" + PS + "d36504431d00d1c6e4d1c34258f2bf0a004de085");
176+
assertEquals(
177+
namesIterator.next().name(),
178+
basedir.toUri() + PS + ".locks" + PS + "fbcebba60d7eb931eca634f6ca494a8a1701b638");
179+
}
180+
}

‎maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapperTest.java‎

Lines changed: 72 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,33 @@
2020

2121
import java.io.File;
2222
import java.io.IOException;
23+
import java.nio.file.Path;
2324
import java.util.Collection;
2425
import java.util.Iterator;
2526

2627
import org.eclipse.aether.artifact.DefaultArtifact;
2728
import org.eclipse.aether.metadata.DefaultMetadata;
2829
import org.eclipse.aether.metadata.Metadata;
2930
import org.eclipse.aether.named.NamedLockKey;
31+
import org.eclipse.aether.util.DirectoryUtils;
3032
import org.junit.jupiter.api.Test;
3133

3234
import static java.util.Collections.emptyList;
3335
import static java.util.Collections.singletonList;
3436
import static org.junit.jupiter.api.Assertions.*;
3537

3638
public class BasedirNameMapperTest extends NameMapperTestSupport {
37-
private final String PS = "/"; // we work with URIs now, not OS file paths
38-
39-
BasedirNameMapper mapper = new BasedirNameMapper(new HashingNameMapper(GAVNameMapper.gav()));
39+
private final BasedirNameMapper mapper = new BasedirNameMapper(GAVNameMapper.fileGav());
40+
41+
private String getPrefix() throws IOException {
42+
Path basedir = DirectoryUtils.resolveDirectory(
43+
session, BasedirNameMapper.DEFAULT_LOCKS_DIR, BasedirNameMapper.CONFIG_PROP_LOCKS_DIR, false);
44+
String basedirPath = basedir.toAbsolutePath().toUri().toASCIIString();
45+
if (!basedirPath.endsWith("/")) {
46+
basedirPath = basedirPath + "/";
47+
}
48+
return basedirPath;
49+
}
4050

4151
@Test
4252
void nullsAndEmptyInputs() {
@@ -56,27 +66,27 @@ void nullsAndEmptyInputs() {
5666
}
5767

5868
@Test
59-
void defaultLocksDir() {
69+
void defaultLocksDir() throws IOException {
6070
configProperties.put("aether.syncContext.named.hashing.depth", "0");
6171
configProperties.put("aether.syncContext.named.basedir.locksDir", null);
6272
DefaultArtifact artifact = new DefaultArtifact("group:artifact:1.0");
6373
Collection<NamedLockKey> names = mapper.nameLocks(session, singletonList(artifact), null);
6474
assertEquals(1, names.size());
6575
assertEquals(
66-
names.iterator().next().name(),
67-
basedir.toUri() + PS + ".locks" + PS + "46e98183d232f1e16f863025080c7f2b9797fd10");
76+
getPrefix() + "artifact~group~artifact~1.0.lock",
77+
names.iterator().next().name());
6878
}
6979

7080
@Test
71-
void relativeLocksDir() {
81+
void relativeLocksDir() throws IOException {
7282
configProperties.put("aether.syncContext.named.hashing.depth", "0");
7383
configProperties.put("aether.syncContext.named.basedir.locksDir", "my/locks");
7484
DefaultArtifact artifact = new DefaultArtifact("group:artifact:1.0");
7585
Collection<NamedLockKey> names = mapper.nameLocks(session, singletonList(artifact), null);
7686
assertEquals(1, names.size());
7787
assertEquals(
78-
names.iterator().next().name(),
79-
basedir.toUri() + PS + "my" + PS + "locks" + PS + "46e98183d232f1e16f863025080c7f2b9797fd10");
88+
getPrefix() + "artifact~group~artifact~1.0.lock",
89+
names.iterator().next().name());
8090
}
8191

8292
@Test
@@ -90,52 +100,92 @@ void absoluteLocksDir() throws IOException {
90100
DefaultArtifact artifact = new DefaultArtifact("group:artifact:1.0");
91101
Collection<NamedLockKey> names = mapper.nameLocks(session, singletonList(artifact), null);
92102
assertEquals(1, names.size());
93-
assertEquals(names.iterator().next().name(), customBaseDir + PS + "46e98183d232f1e16f863025080c7f2b9797fd10");
103+
assertEquals(
104+
getPrefix() + "artifact~group~artifact~1.0.lock",
105+
names.iterator().next().name());
94106
}
95107

96108
@Test
97-
void singleArtifact() {
109+
void singleArtifact() throws IOException {
98110
configProperties.put("aether.syncContext.named.hashing.depth", "0");
99111

100112
DefaultArtifact artifact = new DefaultArtifact("group:artifact:1.0");
101113
Collection<NamedLockKey> names = mapper.nameLocks(session, singletonList(artifact), null);
102114
assertEquals(1, names.size());
103115
assertEquals(
104-
names.iterator().next().name(),
105-
basedir.toUri() + PS + ".locks" + PS + "46e98183d232f1e16f863025080c7f2b9797fd10");
116+
getPrefix() + "artifact~group~artifact~1.0.lock",
117+
names.iterator().next().name());
106118
}
107119

108120
@Test
109-
void singleMetadata() {
121+
void singleMetadata() throws IOException {
110122
configProperties.put("aether.syncContext.named.hashing.depth", "0");
111123

112124
DefaultMetadata metadata =
113125
new DefaultMetadata("group", "artifact", "maven-metadata.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT);
114126
Collection<NamedLockKey> names = mapper.nameLocks(session, null, singletonList(metadata));
115127
assertEquals(1, names.size());
116128
assertEquals(
117-
names.iterator().next().name(),
118-
basedir.toUri() + PS + ".locks" + PS + "293b3990971f4b4b02b220620d2538eaac5f221b");
129+
getPrefix() + "metadata~group~artifact.lock",
130+
names.iterator().next().name());
131+
}
132+
133+
@Test
134+
void prefixMetadata() throws IOException {
135+
configProperties.put("aether.syncContext.named.hashing.depth", "0");
136+
137+
DefaultMetadata metadata =
138+
new DefaultMetadata("", "", ".meta/prefixes-central.txt", Metadata.Nature.RELEASE_OR_SNAPSHOT);
139+
Collection<NamedLockKey> names = mapper.nameLocks(session, null, singletonList(metadata));
140+
assertEquals(1, names.size());
141+
assertEquals(
142+
getPrefix() + "metadata~.meta-SLASH-prefixes-central.txt.lock",
143+
names.iterator().next().name());
144+
}
145+
146+
@Test
147+
void rootSomeMetadata() throws IOException {
148+
configProperties.put("aether.syncContext.named.hashing.depth", "0");
149+
150+
DefaultMetadata metadata = new DefaultMetadata("", "", "something.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT);
151+
Collection<NamedLockKey> names = mapper.nameLocks(session, null, singletonList(metadata));
152+
assertEquals(1, names.size());
153+
assertEquals(
154+
getPrefix() + "metadata~something.xml.lock",
155+
names.iterator().next().name());
156+
}
157+
158+
@Test
159+
void nonRootSomeMetadata() throws IOException {
160+
configProperties.put("aether.syncContext.named.hashing.depth", "0");
161+
162+
DefaultMetadata metadata =
163+
new DefaultMetadata("groupId", "artifactId", "something.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT);
164+
Collection<NamedLockKey> names = mapper.nameLocks(session, null, singletonList(metadata));
165+
assertEquals(1, names.size());
166+
assertEquals(
167+
getPrefix() + "metadata~groupId~artifactId~something.xml.lock",
168+
names.iterator().next().name());
119169
}
120170

121171
@Test
122-
void oneAndOne() {
172+
void oneAndOne() throws IOException {
123173
configProperties.put("aether.syncContext.named.hashing.depth", "0");
124174

125175
DefaultArtifact artifact = new DefaultArtifact("agroup:artifact:1.0");
126176
DefaultMetadata metadata =
127177
new DefaultMetadata("bgroup", "artifact", "maven-metadata.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT);
128178
Collection<NamedLockKey> names = mapper.nameLocks(session, singletonList(artifact), singletonList(metadata));
129179

130-
assertEquals(names.size(), 2);
180+
assertEquals(2, names.size());
131181
Iterator<NamedLockKey> namesIterator = names.iterator();
132182

133183
// they are sorted as well
134184
assertEquals(
135-
namesIterator.next().name(),
136-
basedir.toUri() + PS + ".locks" + PS + "d36504431d00d1c6e4d1c34258f2bf0a004de085");
185+
getPrefix() + "artifact~agroup~artifact~1.0.lock",
186+
namesIterator.next().name());
137187
assertEquals(
138-
namesIterator.next().name(),
139-
basedir.toUri() + PS + ".locks" + PS + "fbcebba60d7eb931eca634f6ca494a8a1701b638");
188+
getPrefix() + "metadata~bgroup~artifact.lock",
189+
namesIterator.next().name());
140190
}
141191
}

0 commit comments

Comments
 (0)