Skip to content

Commit 0cd43cc

Browse files
sichapmanSimon Chapman
sichapman
authored and
Simon Chapman
committed
Use API to retrieve project/group avatar where possible
Fixes JENKINS-64814
1 parent 6f19df3 commit 0cd43cc

File tree

7 files changed

+240
-48
lines changed

7 files changed

+240
-48
lines changed

‎src/main/java/io/jenkins/plugins/gitlabbranchsource/GitLabSCMNavigator.java‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,11 @@ public class GitLabSCMNavigator extends SCMNavigator {
9393
* The GitLab server name configured in Jenkins.
9494
*/
9595
private String serverName;
96+
97+
/**
98+
* The full version of the GitLab server.
99+
*/
100+
private String serverVersion;
96101
/**
97102
* The default credentials to use for checking out).
98103
*/
@@ -206,6 +211,11 @@ private GitLabOwner getGitlabOwner(SCMNavigatorOwner owner) {
206211
private GitLabOwner getGitlabOwner(GitLabApi gitLabApi) {
207212
if (gitlabOwner == null) {
208213
gitlabOwner = GitLabOwner.fetchOwner(gitLabApi, projectOwner);
214+
try {
215+
serverVersion = gitLabApi.getVersion().getVersion();
216+
} catch (GitLabApiException e) {
217+
serverVersion = "0.0";
218+
}
209219
}
210220
return gitlabOwner;
211221
}
@@ -417,7 +427,11 @@ protected List<Action> retrieveActions(
417427
List<Action> result = new ArrayList<>();
418428
result.add(new ObjectMetadataAction(Util.fixEmpty(fullName), description, webUrl));
419429
if (StringUtils.isNotBlank(avatarUrl)) {
420-
result.add(new GitLabAvatar(avatarUrl));
430+
if (GitLabServer.groupAvatarsApiAvailable(serverVersion)) {
431+
result.add(new GitLabAvatar(avatarUrl, serverName, projectOwner, false));
432+
} else {
433+
result.add(new GitLabAvatar(avatarUrl));
434+
}
421435
}
422436
result.add(GitLabLink.toGroup(webUrl));
423437
if (StringUtils.isBlank(webUrl)) {

‎src/main/java/io/jenkins/plugins/gitlabbranchsource/GitLabSCMSource.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ public class GitLabSCMSource extends AbstractGitSCMSource {
108108

109109
public static final Logger LOGGER = Logger.getLogger(GitLabSCMSource.class.getName());
110110
private final String serverName;
111+
private String serverVersion;
111112
private final String projectOwner;
112113
private final String projectPath;
113114
private String projectName;
@@ -218,6 +219,11 @@ protected Project getGitlabProject(GitLabApi gitLabApi) {
218219
} catch (GitLabApiException e) {
219220
throw new IllegalStateException("Failed to retrieve project " + projectPath, e);
220221
}
222+
try {
223+
serverVersion = gitLabApi.getVersion().getVersion();
224+
} catch (GitLabApiException e) {
225+
serverVersion = "0.0";
226+
}
221227
}
222228
return gitlabProject;
223229
}
@@ -616,7 +622,11 @@ protected List<Action> retrieveActions(SCMSourceEvent event, @NonNull TaskListen
616622
result.add(new ObjectMetadataAction(name, gitlabProject.getDescription(), projectUrl));
617623
String avatarUrl = gitlabProject.getAvatarUrl();
618624
if (!ctx.projectAvatarDisabled() && StringUtils.isNotBlank(avatarUrl)) {
619-
result.add(new GitLabAvatar(avatarUrl));
625+
if (GitLabServer.projectAvatarsApiAvailable(serverVersion)) {
626+
result.add(new GitLabAvatar(avatarUrl, serverName, projectPath, true));
627+
} else {
628+
result.add(new GitLabAvatar(avatarUrl));
629+
}
620630
}
621631
result.add(GitLabLink.toProject(projectUrl));
622632
return result;

‎src/main/java/io/jenkins/plugins/gitlabbranchsource/helpers/GitLabAvatar.java‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,36 @@
11
package io.jenkins.plugins.gitlabbranchsource.helpers;
22

33
import edu.umd.cs.findbugs.annotations.NonNull;
4+
import io.jenkins.cli.shaded.org.apache.commons.lang.StringUtils;
45
import java.util.Objects;
56
import jenkins.scm.api.metadata.AvatarMetadataAction;
6-
import org.apache.commons.lang.StringUtils;
77

88
public class GitLabAvatar extends AvatarMetadataAction {
99

10+
private final GitLabAvatarLocation location;
11+
12+
/**
13+
* Back compat, to keep existing configs working upon plugin upgrade
14+
*/
1015
private final String avatar;
1116

12-
public GitLabAvatar(String avatar) {
13-
this.avatar = avatar;
17+
public GitLabAvatar(String avatarUrl, String serverName, String projectPath, boolean isProject) {
18+
this.avatar = null;
19+
this.location = new GitLabAvatarLocation(avatarUrl, serverName, projectPath, isProject);
20+
}
21+
22+
public GitLabAvatar(String avatarUrl) {
23+
this.avatar = null;
24+
this.location = new GitLabAvatarLocation(avatarUrl);
1425
}
1526

1627
@Override
1728
public String getAvatarImageOf(@NonNull String size) {
18-
return StringUtils.isBlank(avatar) ? null : GitLabAvatarCache.buildUrl(avatar, size);
29+
if (StringUtils.isNotBlank(avatar)) {
30+
// Back compat, to keep existing configs working upon plugin upgrade
31+
return GitLabAvatarCache.buildUrl(new GitLabAvatarLocation(avatar), size);
32+
}
33+
return location != null && location.available() ? GitLabAvatarCache.buildUrl(location, size) : null;
1934
}
2035

2136
@Override
@@ -29,11 +44,11 @@ public boolean equals(Object o) {
2944

3045
GitLabAvatar that = (GitLabAvatar) o;
3146

32-
return Objects.equals(avatar, that.avatar);
47+
return Objects.equals(location, that.location);
3348
}
3449

3550
@Override
3651
public int hashCode() {
37-
return avatar != null ? avatar.hashCode() : 0;
52+
return location != null ? location.hashCode() : 0;
3853
}
3954
}

‎src/main/java/io/jenkins/plugins/gitlabbranchsource/helpers/GitLabAvatarCache.java‎

Lines changed: 51 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package io.jenkins.plugins.gitlabbranchsource.helpers;
22

3+
import static io.jenkins.plugins.gitlabbranchsource.helpers.GitLabHelper.apiBuilderNoAccessControl;
34
import static java.awt.RenderingHints.KEY_ALPHA_INTERPOLATION;
45
import static java.awt.RenderingHints.KEY_INTERPOLATION;
56
import static java.awt.RenderingHints.VALUE_ALPHA_INTERPOLATION_QUALITY;
@@ -47,6 +48,7 @@
4748
import javax.servlet.http.HttpServletResponse;
4849
import jenkins.model.Jenkins;
4950
import org.apache.commons.lang.StringUtils;
51+
import org.gitlab4j.api.GitLabApi;
5052
import org.kohsuke.stapler.HttpResponse;
5153
import org.kohsuke.stapler.QueryParameter;
5254
import org.kohsuke.stapler.StaplerRequest;
@@ -94,19 +96,18 @@ public GitLabAvatarCache() {}
9496
/**
9597
* Builds the URL for the cached avatar image of the required size.
9698
*
97-
* @param url the URL of the source avatar image.
98-
* @param size the size of the image.
99-
* @return the URL of the cached image.
99+
* @param location the external endpoint details (url/API)
100+
* @return the Jenkins URL of the cached image.
100101
*/
101-
public static String buildUrl(String url, String size) {
102+
public static String buildUrl(GitLabAvatarLocation location, String size) {
102103
Jenkins j = Jenkins.get();
103104
GitLabAvatarCache instance = j.getExtensionList(RootAction.class).get(GitLabAvatarCache.class);
104105
if (instance == null) {
105106
throw new AssertionError();
106107
}
107-
String key = Util.getDigestOf(GitLabAvatarCache.class.getName() + url);
108+
String key = Util.getDigestOf(GitLabAvatarCache.class.getName() + location.toString());
108109
// seed the cache
109-
instance.getCacheEntry(key, url);
110+
instance.getCacheEntry(key, location);
110111
return UriTemplate.buildFromTemplate(j.getRootUrlFromRequest())
111112
.literal(instance.getUrlName())
112113
.path("key")
@@ -283,12 +284,10 @@ public HttpResponse doDynamic(StaplerRequest req, @QueryParameter String size) {
283284
}
284285
}
285286
final CacheEntry avatar = getCacheEntry(key, null);
286-
if (avatar == null || !(avatar.url.startsWith("http://") || avatar.url.startsWith("https://"))) {
287-
// we will generate avatars if the URL is not HTTP based
288-
// since the url string will not magically turn itself into a HTTP url this
289-
// avatar is immutable
287+
if (avatar == null) {
288+
// we will generate immutable avatars if cache did not get seeded for some reason
290289
return new ImageResponse(
291-
generateAvatar(avatar == null ? "" : avatar.url, targetSize),
290+
generateAvatar("", targetSize),
292291
true,
293292
System.currentTimeMillis(),
294293
"max-age=365000000, immutable, public");
@@ -297,7 +296,8 @@ public HttpResponse doDynamic(StaplerRequest req, @QueryParameter String size) {
297296
// serve a temporary avatar until we get the remote one, no caching as we could
298297
// have the real deal
299298
// real soon now
300-
return new ImageResponse(generateAvatar(avatar.url, targetSize), true, -1L, "no-cache, public");
299+
return new ImageResponse(
300+
generateAvatar(avatar.avatarLocation.toString(), targetSize), true, -1L, "no-cache, public");
301301
}
302302
long since = req.getDateHeader("If-Modified-Since");
303303
if (avatar.lastModified <= since) {
@@ -313,7 +313,8 @@ public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object nod
313313
}
314314
if (avatar.image == null) {
315315
// we can retry in an hour
316-
return new ImageResponse(generateAvatar(avatar.url, targetSize), true, -1L, "max-age=3600, public");
316+
return new ImageResponse(
317+
generateAvatar(avatar.avatarLocation.toString(), targetSize), true, -1L, "max-age=3600, public");
317318
}
318319

319320
BufferedImage image = avatar.image;
@@ -329,30 +330,30 @@ public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object nod
329330
* Retrieves the entry from the cache.
330331
*
331332
* @param key the cache key.
332-
* @param url the URL to fetch if the entry is missing or {@code null} to
333+
* @param avatarLocation the location details to fetch the avatar from or {@code null} to
333334
* perform a read-only check.
334335
* @return the entry or {@code null} if a read-only check found no matching
335336
* entry.
336337
*/
337338
@Nullable
338-
private CacheEntry getCacheEntry(@NonNull final String key, @Nullable final String url) {
339+
private CacheEntry getCacheEntry(@NonNull final String key, @Nullable final GitLabAvatarLocation avatarLocation) {
339340
CacheEntry entry = cache.get(key);
340341
if (entry == null) {
341342
synchronized (serviceLock) {
342343
entry = cache.get(key);
343344
if (entry == null) {
344-
if (url == null) {
345+
if (avatarLocation == null) {
345346
return null;
346347
}
347-
entry = new CacheEntry(url, service.submit(new FetchImage(url)));
348+
entry = new CacheEntry(avatarLocation, service.submit(new FetchImage(avatarLocation)));
348349
cache.put(key, entry);
349350
}
350351
}
351352
} else {
352353
if (entry.isStale()) {
353354
synchronized (serviceLock) {
354355
if (!entry.pending()) {
355-
entry.setFuture(service.submit(new FetchImage(entry.url)));
356+
entry.setFuture(service.submit(new FetchImage(entry.avatarLocation)));
356357
}
357358
}
358359
}
@@ -381,15 +382,15 @@ private CacheEntry getCacheEntry(@NonNull final String key, @Nullable final Stri
381382
}
382383

383384
private static class CacheEntry {
384-
private final String url;
385+
private GitLabAvatarLocation avatarLocation;
385386
private BufferedImage image;
386387
private long lastModified;
387388
private long lastAccessed = -1L;
388389

389390
private Future<CacheEntry> future;
390391

391-
public CacheEntry(String url, BufferedImage image, long lastModified) {
392-
this.url = url;
392+
public CacheEntry(GitLabAvatarLocation avatarLocation, BufferedImage image, long lastModified) {
393+
this.avatarLocation = avatarLocation;
393394
if (image.getHeight() > 128 || image.getWidth() > 128) {
394395
// limit the amount of storage
395396
this.image = scaleImage(image, 128);
@@ -400,15 +401,15 @@ public CacheEntry(String url, BufferedImage image, long lastModified) {
400401
this.lastModified = lastModified < 0 ? System.currentTimeMillis() : lastModified;
401402
}
402403

403-
public CacheEntry(String url, Future<CacheEntry> future) {
404-
this.url = url;
404+
public CacheEntry(GitLabAvatarLocation avatarLocation, Future<CacheEntry> future) {
405+
this.avatarLocation = avatarLocation;
405406
this.image = null;
406407
this.lastModified = System.currentTimeMillis();
407408
this.future = future;
408409
}
409410

410-
public CacheEntry(String url) {
411-
this.url = url;
411+
public CacheEntry(GitLabAvatarLocation avatarLocation) {
412+
this.avatarLocation = avatarLocation;
412413
this.lastModified = System.currentTimeMillis();
413414
}
414415

@@ -426,6 +427,7 @@ public synchronized boolean pending() {
426427
image = pending.image;
427428
}
428429
lastModified = pending.lastModified;
430+
avatarLocation = pending.avatarLocation;
429431
future = null;
430432
return false;
431433
} catch (InterruptedException | ExecutionException e) {
@@ -489,23 +491,37 @@ public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object nod
489491
}
490492

491493
private static class FetchImage implements Callable<CacheEntry> {
492-
private final String url;
494+
private final GitLabAvatarLocation avatarLocation;
493495

494-
public FetchImage(String url) {
495-
this.url = url;
496+
public FetchImage(GitLabAvatarLocation avatarLocation) {
497+
this.avatarLocation = avatarLocation;
496498
}
497499

498500
@Override
499501
public CacheEntry call() throws Exception {
500-
LOGGER.log(Level.FINE, "Attempting to fetch remote avatar: {0}", url);
502+
LOGGER.log(Level.FINE, "Attempting to fetch remote avatar: {0}", avatarLocation.toString());
501503
long start = System.nanoTime();
502504
try {
503-
HttpURLConnection connection = (HttpURLConnection) new URL(url).openConnection();
505+
if (avatarLocation.apiAvailable()) {
506+
try (GitLabApi apiClient = apiBuilderNoAccessControl(avatarLocation.getServerName());
507+
InputStream is = avatarLocation.isProject()
508+
? apiClient.getProjectApi().getAvatar(avatarLocation.getFullPath())
509+
: apiClient.getGroupApi().getAvatar(avatarLocation.getFullPath())) {
510+
BufferedImage image = ImageIO.read(is);
511+
if (image == null) {
512+
return new CacheEntry(avatarLocation);
513+
}
514+
return new CacheEntry(avatarLocation, image, -1);
515+
}
516+
}
517+
518+
HttpURLConnection connection =
519+
(HttpURLConnection) new URL(avatarLocation.getAvatarUrl()).openConnection();
504520
try {
505521
connection.setConnectTimeout(10000);
506522
connection.setReadTimeout(30000);
507523
if (!connection.getContentType().startsWith("image/")) {
508-
return new CacheEntry(url);
524+
return new CacheEntry(avatarLocation);
509525
}
510526
int length = connection.getContentLength();
511527
// buffered stream should be no more than 16k if we know the length
@@ -515,21 +531,21 @@ public CacheEntry call() throws Exception {
515531
BufferedInputStream bis = new BufferedInputStream(is, length)) {
516532
BufferedImage image = ImageIO.read(bis);
517533
if (image == null) {
518-
return new CacheEntry(url);
534+
return new CacheEntry(avatarLocation);
519535
}
520-
return new CacheEntry(url, image, connection.getLastModified());
536+
return new CacheEntry(avatarLocation, image, connection.getLastModified());
521537
}
522538
} finally {
523539
connection.disconnect();
524540
}
525541
} catch (IOException e) {
526542
LOGGER.log(Level.INFO, e.getMessage(), e);
527-
return new CacheEntry(url);
543+
return new CacheEntry(avatarLocation);
528544
} finally {
529545
long end = System.nanoTime();
530546
long duration = TimeUnit.NANOSECONDS.toMillis(end - start);
531547
LOGGER.log(duration > 250 ? Level.INFO : Level.FINE, "Avatar lookup of {0} took {1}ms", new Object[] {
532-
url, duration
548+
avatarLocation.toString(), duration
533549
});
534550
}
535551
}

0 commit comments

Comments
 (0)