Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jenkinsci.plugins.displayurlapi;

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.Util;
import hudson.model.Job;
Expand All @@ -12,26 +13,31 @@
public class ClassicDisplayURLProvider extends DisplayURLProvider {

@Override
@NonNull
public String getDisplayName() {
return Messages.classic_name();
}

@Override
@NonNull
public String getName() {
return "classic";
}

@Override
@NonNull
public String getRunURL(Run<?, ?> run) {
return getRoot() + Util.encode(run.getUrl());
}

@Override
@NonNull
public String getChangesURL(Run<?, ?> run) {
return getJobURL(run.getParent()) + "changes";
}

@Override
@NonNull
public String getJobURL(Job<?, ?> job) {
return getRoot() + Util.encode(job.getUrl());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ public class DisplayURLContext implements Closeable {
/**
* The current thread's context.
*/
private static ThreadLocal<Stack<DisplayURLContext>> context = new ThreadLocal<Stack<DisplayURLContext>>();
private static ThreadLocal<Stack<DisplayURLContext>> context = new ThreadLocal<>();
/**
* Class names that we expect to be in the stack trace for calls to {@link #open()}.
*/
private static Set<String> ourPluginClassNames = new HashSet<String>(Arrays.asList(
private static Set<String> ourPluginClassNames = new HashSet<>(Arrays.asList(
DisplayURLContext.class.getName(),
DisplayURLProvider.class.getName(),
DisplayURLProvider.DisplayURLProviderImpl.class.getName()
Expand Down Expand Up @@ -88,7 +88,7 @@ private DisplayURLContext(DisplayURLContext parent) {
*/
private void guessPlugin() {
StackTraceElement[] stack = (new Throwable()).getStackTrace();
PluginManager manager = Jenkins.getActiveInstance().getPluginManager();
PluginManager manager = Jenkins.getInstance().getPluginManager();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Jenkins.get() already available in the used Jenkins version? Then this should be used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly it is not 😢

ClassLoader loader = manager.uberClassLoader;
for (StackTraceElement frame : stack) {
String cname = frame.getClassName();
Expand Down Expand Up @@ -117,7 +117,7 @@ private void guessPlugin() {
public static DisplayURLContext open() {
Stack<DisplayURLContext> stack = DisplayURLContext.context.get();
if (stack == null) {
stack = new Stack<DisplayURLContext>();
stack = new Stack<>();
DisplayURLContext.context.set(stack);
}
DisplayURLContext context = new DisplayURLContext(stack.isEmpty() ? null : stack.peek());
Expand Down Expand Up @@ -225,7 +225,7 @@ public DisplayURLContext plugin(@CheckForNull PluginWrapper plugin) {
@NonNull
public DisplayURLContext attribute(String name, String value) {
if (this.attributes == null) {
this.attributes = new HashMap<String, String>();
this.attributes = new HashMap<>();
}
this.attributes.put(name, value);
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,23 @@ public abstract class DisplayURLDecorator implements ExtensionPoint {
*/
@NonNull
public static String decorate(@NonNull DisplayURLContext context, @NonNull String url) {
ExtensionList<DisplayURLDecorator> extensionList = ExtensionList.lookup(DisplayURLDecorator.class);
ExtensionList<DisplayURLDecorator> extensionList = ExtensionList
.lookup(DisplayURLDecorator.class);
if (extensionList.isEmpty()) {
return url;
}
Map<String, String> parameters = new TreeMap<String, String>();
Map<String, String> parameters = new TreeMap<>();
// the extension with the highest ordinal wins for duplicate query parameters
for (DisplayURLDecorator decorator : extensionList.reverseView()) {
parameters.putAll(fixNull(decorator.parameters(context)));
}
if (parameters.isEmpty()) {
return url;
}
Map<String, String> encodedParameters = new TreeMap<String, String>();
Map<String, String> encodedParameters = new TreeMap<>();
for (Map.Entry<String, String> p : parameters.entrySet()) {
encodedParameters.put(encode(p.getKey()), p.getValue() == null ? null : encode(p.getValue()));
encodedParameters
.put(encode(p.getKey()), p.getValue() == null ? null : encode(p.getValue()));
}
StringBuilder result = new StringBuilder(2083); // maximum URL length
char sep = '?';
Expand Down Expand Up @@ -97,7 +99,7 @@ public static String decorate(@NonNull DisplayURLContext context, @NonNull Strin
*/
@NonNull
private static <K, V> Map<K, V> fixNull(@CheckForNull Map<K, V> map) {
return map == null ? Collections.<K, V>emptyMap() : map;
return map == null ? Collections.emptyMap() : map;
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jenkinsci.plugins.displayurlapi;

import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.Nullable;
import hudson.ExtensionList;
import hudson.ExtensionPoint;
Expand All @@ -9,15 +10,13 @@
import jenkins.model.Jenkins;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.plugins.displayurlapi.actions.AbstractDisplayAction;
import org.jenkinsci.plugins.displayurlapi.user.PreferredProviderUserProperty;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

import static org.apache.commons.lang.StringUtils.isNotEmpty;

/**
* Generates URLs for well known UI locations for use in notifications (e.g. mailer, HipChat, Slack, IRC, etc)
* Extensible to allow plugins to override common URLs (e.g. Blue Ocean or another future secondary UI)
* Generates URLs for well known UI locations for use in notifications (e.g. mailer, HipChat, Slack,
* IRC, etc) Extensible to allow plugins to override common URLs (e.g. Blue Ocean or another future
* secondary UI)
*/
public abstract class DisplayURLProvider implements ExtensionPoint {

Expand All @@ -35,23 +34,25 @@ public static DisplayURLProvider get() {
*
* @return all the {@link DisplayURLProvider} implementations.
*/
public static Iterable<DisplayURLProvider> all() {
public static ExtensionList<DisplayURLProvider> all() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not breaking since ExtensionList is still Iterable, it just lends itself better to stream API.

Copy link
Member

@dwnusbaum dwnusbaum Sep 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this, it causes problems in environments like the PCT:

java.lang.NoSuchMethodError: org.jenkinsci.plugins.displayurlapi.DisplayURLProvider.all()Ljava/lang/Iterable;
	at org.jenkinsci.plugins.blueoceandisplayurl.BlueOceanDisplayURLImplTest.setUp(BlueOceanDisplayURLImplTest.java:178)

See BlueOceanDisplayURLImplTest. That is the only use of DisplayURLProvider.all() outside of this plugin that I found from a quick search, and it's in test code, so it doesn't seem to be a big deal.

Based on the spec, I don't think this is a compatible change, since JARs compiled against the old class are looking for a method whose return type is Iterable, but no such method exists. Because the new return type is a subtype of the old type, I think the change is source compatible in that all callers can be recompiled against the new version of this plugin without needing to make any changes on their end and things will work correctly, but not binary compatible.

return ExtensionList.lookup(DisplayURLProvider.class);
}

public static DisplayURLProvider getDefault() {
DisplayURLProvider defaultProvider = getPreferredProvider();
if (defaultProvider == null) {
defaultProvider = ExtensionList.lookup(DisplayURLProvider.class).get(ClassicDisplayURLProvider.class);
defaultProvider = ExtensionList.lookup(DisplayURLProvider.class)
.get(ClassicDisplayURLProvider.class);
}
return defaultProvider;
}

/**
* Fully qualified URL for the Root display URL
*/
@NonNull
public String getRoot() {
String root = Jenkins.getActiveInstance().getRootUrl();
String root = Jenkins.getInstance().getRootUrl();
if (root == null) {
root = "http://unconfigured-jenkins-location/";
}
Expand All @@ -61,28 +62,35 @@ public String getRoot() {
/**
* Display name of this provider e.g. "Jenkins Classic", "Blue Ocean", etc
*/
@NonNull
public String getDisplayName() {
return this.getClass().getSimpleName();
}

/** Name of provider to be used as an id. Do not use i18n */
/**
* Name of provider to be used as an id. Do not use i18n
*/
@NonNull
public String getName() {
return this.getClass().getSimpleName();
}

/**
* Fully qualified URL for a Run
*/
@NonNull
public abstract String getRunURL(Run<?, ?> run);

/**
* Fully qualified URL for a page that displays changes for a project.
*/
@NonNull
public abstract String getChangesURL(Run<?, ?> run);

/**
* Fully qualified URL for a Jobs home
*/
@NonNull
public abstract String getJobURL(Job<?, ?> job);

/**
Expand All @@ -96,44 +104,39 @@ static class DisplayURLProviderImpl extends ClassicDisplayURLProvider {
static final String DISPLAY_POSTFIX = AbstractDisplayAction.URL_NAME + "/redirect";

@Override
@NonNull
public String getRunURL(Run<?, ?> run) {
DisplayURLContext ctx = DisplayURLContext.open();
try {
try (DisplayURLContext ctx = DisplayURLContext.open()) {
if (ctx.run() == null) {
// the link might be generated from another run so we only add this to the context if unset
ctx.run(run);
}
return DisplayURLDecorator.decorate(ctx, super.getRunURL(run) + DISPLAY_POSTFIX);
} finally {
ctx.close();
}
}

@Override
@NonNull
public String getChangesURL(Run<?, ?> run) {
DisplayURLContext ctx = DisplayURLContext.open();
try {
try (DisplayURLContext ctx = DisplayURLContext.open()) {
if (ctx.run() == null) {
// the link might be generated from another run so we only add this to the context if unset
ctx.run(run);
}
return DisplayURLDecorator.decorate(ctx, super.getRunURL(run) + DISPLAY_POSTFIX + "?page=changes");
} finally {
ctx.close();
return DisplayURLDecorator
.decorate(ctx, super.getRunURL(run) + DISPLAY_POSTFIX + "?page=changes");
}
}

@Override
@NonNull
public String getJobURL(Job<?, ?> job) {
DisplayURLContext ctx = DisplayURLContext.open();
try {
try (DisplayURLContext ctx = DisplayURLContext.open()) {
if (ctx.job() == null) {
// the link might be generated from another job so we only add this to the context if unset
ctx.job(job);
}
return DisplayURLDecorator.decorate(ctx, super.getJobURL(job) + DISPLAY_POSTFIX);
} finally {
ctx.close();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,40 +1,35 @@
package org.jenkinsci.plugins.displayurlapi;

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.EnvVars;
import hudson.Extension;
import hudson.model.EnvironmentContributor;
import hudson.model.Job;
import hudson.model.Run;
import hudson.model.TaskListener;

import javax.annotation.Nonnull;
import java.io.IOException;

@Extension
public class EnvironmentContributorImpl extends EnvironmentContributor {

@Override
public void buildEnvironmentFor(@Nonnull Run r, @Nonnull EnvVars envs, @Nonnull TaskListener listener) throws IOException, InterruptedException {
DisplayURLContext ctx = DisplayURLContext.open();
try {
public void buildEnvironmentFor(@NonNull Run r, @NonNull EnvVars envs, @NonNull TaskListener listener) throws IOException, InterruptedException {
try (DisplayURLContext ctx = DisplayURLContext.open()) {
ctx.run(r);
ctx.plugin(null); // environment contributor "comes from" core
DisplayURLProvider urlProvider = DisplayURLProvider.get();
envs.put("RUN_DISPLAY_URL", urlProvider.getRunURL(r));
envs.put("RUN_CHANGES_DISPLAY_URL", urlProvider.getChangesURL(r));
} finally {
ctx.close();
}
}

@Override
public void buildEnvironmentFor(@Nonnull Job j, @Nonnull EnvVars envs, @Nonnull TaskListener listener) throws IOException, InterruptedException {
DisplayURLContext ctx = DisplayURLContext.open();
try {
public void buildEnvironmentFor(@NonNull Job j, @NonNull EnvVars envs, @NonNull TaskListener listener) throws IOException, InterruptedException {
try (DisplayURLContext ctx = DisplayURLContext.open()) {
ctx.job(j);
ctx.plugin(null); // environment contributor "comes from" core
envs.put("JOB_DISPLAY_URL", DisplayURLProvider.get().getJobURL(j));
} finally {
ctx.close();
}
}
}
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
package org.jenkinsci.plugins.displayurlapi.actions;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
import com.google.common.collect.Iterables;
import hudson.ExtensionList;
import hudson.model.Action;
import hudson.model.User;
import java.util.Objects;
import java.util.function.Predicate;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.plugins.displayurlapi.ClassicDisplayURLProvider;
import org.jenkinsci.plugins.displayurlapi.DisplayURLProvider;
import org.jenkinsci.plugins.displayurlapi.user.PreferredProviderUserProperty;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;

import javax.annotation.Nullable;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;

Expand Down Expand Up @@ -45,27 +45,22 @@ public final Object doRedirect(StaplerRequest req, StaplerResponse rsp) throws I

DisplayURLProvider lookupProvider(StaplerRequest req) {
final String providerName = req.getParameter("provider");
if(providerName != null && !providerName.isEmpty()) {
Iterable<DisplayURLProvider> providers = DisplayURLProvider.all();
Iterable<DisplayURLProvider> filtered = Iterables.filter(providers, new Predicate<DisplayURLProvider>() {
@Override
public boolean apply(@Nullable DisplayURLProvider displayURLProvider) {
if(displayURLProvider == null) {
return false;
}

return displayURLProvider.getName().equals(providerName);
}
});

DisplayURLProvider provider = Iterables.getFirst(filtered, null);
if(provider != null) {
if (StringUtils.isNotEmpty(providerName)) {
ExtensionList<DisplayURLProvider> providers = DisplayURLProvider.all();
DisplayURLProvider provider = providers.stream()
.filter(Objects::nonNull)
.filter(displayURLProvider -> providerName.equals(displayURLProvider.getName()))
.findFirst()
.orElse(null);

if (provider != null) {
return provider;
}
}

return lookupProvider();
}

DisplayURLProvider lookupProvider() {
PreferredProviderUserProperty prefProperty = getUserPreferredProviderProperty();

Expand All @@ -74,9 +69,12 @@ DisplayURLProvider lookupProvider() {
}
DisplayURLProvider displayURLProvider = DisplayURLProvider.getPreferredProvider();
if (displayURLProvider == null) {
Iterable<DisplayURLProvider> all = DisplayURLProvider.all();
Iterable<DisplayURLProvider> availableProviders = Iterables.filter(all, Predicates.not(Predicates.instanceOf(ClassicDisplayURLProvider.class)));
displayURLProvider = Iterables.getFirst(availableProviders, DisplayURLProvider.getDefault());
ExtensionList<DisplayURLProvider> all = DisplayURLProvider.all();
displayURLProvider = all.stream()
.filter(
((Predicate<DisplayURLProvider>) ClassicDisplayURLProvider.class::isInstance)
.negate())
.findFirst().orElse(DisplayURLProvider.getDefault());
}
return displayURLProvider;
}
Expand Down
Loading