diff --git a/src/main/java/hudson/plugins/favorite/FavoritePlugin.java b/src/main/java/hudson/plugins/favorite/FavoritePlugin.java index 3b81abd..34b9d23 100644 --- a/src/main/java/hudson/plugins/favorite/FavoritePlugin.java +++ b/src/main/java/hudson/plugins/favorite/FavoritePlugin.java @@ -7,6 +7,7 @@ import jenkins.model.Jenkins; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; +import org.kohsuke.stapler.HttpResponses; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; @@ -19,47 +20,26 @@ public class FavoritePlugin extends Plugin { @RequirePOST public void doToggleFavorite(StaplerRequest req, StaplerResponse resp, @QueryParameter String job, @QueryParameter Boolean redirect) throws IOException { - Jenkins jenkins = Jenkins.get(); User user = User.current(); if (user != null) { - try { - Favorites.toggleFavorite(user, getItem(job)); - } catch (FavoriteException e) { - throw new IOException(e); + Jenkins jenkins = Jenkins.get(); + Item item = jenkins.getItemByFullName(job); + if (item == null) { + throw HttpResponses.notFound(); } - } - if(redirect) { - if (!job.contains("/")) - { - // Works for default URL pattern: rootUrl/job/jobName - resp.sendRedirect(resp.encodeRedirectURL(jenkins.getRootUrl() + "job/" + job)); + + try { + Favorites.toggleFavorite(user, item); + } catch (FavoriteException fe) { + throw HttpResponses.error(fe); } - else - { - // Works for folder URL pattern: - // rootUrl/job/folder/job/jobName - // rootUrl/job/folder/job/subfolder/job/jobName etc. - StringBuilder urlPostfix = new StringBuilder("job/"); - String[] itemNames = job.split("/"); - for (int i = 0; i < itemNames.length; i++) - { - urlPostfix.append(itemNames[i]); - if (i < itemNames.length - 1) - { - urlPostfix.append("/job/"); - } - } - resp.sendRedirect(resp.encodeRedirectURL(jenkins.getRootUrl() + urlPostfix.toString())); + + if (redirect) { + resp.sendRedirect(jenkins.getRootUrl() + item.getUrl()); } + } else { + throw HttpResponses.forbidden(); } } - public static Item getItem(String fullName) { - Jenkins jenkins = Jenkins.get(); - Item item = jenkins.getItemByFullName(fullName); - if (item == null) { - throw new IllegalArgumentException("Item <" + fullName + "> does not exist"); - } - return item; - } } diff --git a/src/test/java/hudson/plugins/favorite/FavoritesPluginTest.java b/src/test/java/hudson/plugins/favorite/FavoritesPluginTest.java index d6fe655..1c7b18e 100644 --- a/src/test/java/hudson/plugins/favorite/FavoritesPluginTest.java +++ b/src/test/java/hudson/plugins/favorite/FavoritesPluginTest.java @@ -28,7 +28,7 @@ public class FavoritesPluginTest { - private static List events = new CopyOnWriteArrayList<>(); + private static final List events = new CopyOnWriteArrayList<>(); @Rule public JenkinsRule rule = new JenkinsRule(); @@ -47,9 +47,7 @@ public void testToggleFavoriteAnonymous() throws Exception { rule.createFreeStyleProject("cats"); try (JenkinsRule.WebClient client = rule.createWebClient()) { WebResponse response = requestToggleFavorite(client, "cats", false); - // TODO should return 403 (pr to follow) - //assertEquals(403, response.getStatusCode()); - assertEquals(200, response.getStatusCode()); + assertEquals(403, response.getStatusCode()); // verify favorite column not visible HtmlPage root = client.goTo(""); @@ -91,8 +89,7 @@ public void testToggleFavorite() throws Exception { assertFalse(Favorites.isFavorite(bob, cats)); } - // TODO fix favorite listener issue (pr to follow) - //assertEquals(Arrays.asList("add:bob:cats", "remove:bob:cats"), events); + assertEquals(Arrays.asList("add:bob:cats", "remove:bob:cats"), events); } @Test @@ -100,11 +97,7 @@ public void testToggleFavoriteJobDoesNotExist() throws Exception { try (JenkinsRule.WebClient client = rule.createWebClient()) { client.login("bob", "bob"); WebResponse res = requestToggleFavorite(client, "foo/bar", false); - - // TODO should return 404 (pr to follow) - //assertEquals(404, res.getStatusCode()); - assertEquals(500, res.getStatusCode()); - + assertEquals(404, res.getStatusCode()); } assertEquals(0, events.size());