Skip to content
Permalink
Browse files
[JENKINS-42717] - Prevent NPE when a non-existent Default View is spe…
…cified in the global config (#2815)

* [JENKINS-42717] - Document view management methods in Jenkins and ViewGroupMixIn

* [KENKINS-42717] - GlobalDefauldViewConfiguration should not fail with NPE when the view is missing

* [JENKINS-42717] - Draft the direct unit test

* [JENKINS-42717] - Fix the tes implementation

* [JENKINS-42717] - Make FormException localizable

* [JENKINS-42717] - Fix te build glitch

(cherry picked from commit 4074818)
  • Loading branch information
oleg-nenashev authored and olivergondza committed May 2, 2017
1 parent 161095e commit 0e2cadf5ea0a37780718207fab1e077a32127162
@@ -35,6 +35,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import javax.annotation.CheckForNull;

/**
* Implements {@link ViewGroup} to be used as a "mix-in".
@@ -91,7 +92,14 @@ public synchronized void deleteView(View view) throws IOException {
owner.save();
}

public View getView(String name) {
/**
* Gets a view by the specified name.
* The method iterates through {@link ViewGroup}s if required.
* @param name Name of the view
* @return View instance or {@code null} if it is missing
*/
@CheckForNull
public View getView(@CheckForNull String name) {
for (View v : views()) {
if(v.getViewName().equals(name))
return v;
@@ -24,6 +24,7 @@
package hudson.views;

import hudson.Extension;
import hudson.model.View;
import jenkins.model.GlobalConfiguration;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
@@ -41,7 +42,18 @@
public boolean configure(StaplerRequest req, JSONObject json) throws FormException {
// for compatibility reasons, the actual value is stored in Jenkins
Jenkins j = Jenkins.getInstance();
j.setPrimaryView(json.has("primaryView") ? j.getView(json.getString("primaryView")) : j.getViews().iterator().next());
if (json.has("primaryView")) {
final String viewName = json.getString("primaryView");
final View newPrimaryView = j.getView(viewName);
if (newPrimaryView == null) {
throw new FormException(Messages.GlobalDefaultViewConfiguration_ViewDoesNotExist(viewName), "primaryView");
}
j.setPrimaryView(newPrimaryView);
} else {
// Fallback if the view is not specified
j.setPrimaryView(j.getViews().iterator().next());
}

return true;
}
}
@@ -1825,7 +1825,14 @@ public String getFullDisplayName() {
return names;
}

public View getView(String name) {
/**
* Gets a view by the specified name.
* The method iterates through {@link ViewGroup}s if required.
* @param name Name of the view
* @return View instance or {@code null} if it is missing
*/
@CheckForNull
public View getView(@CheckForNull String name) {
return viewGroupMixIn.getView(name);
}

@@ -1884,7 +1891,7 @@ public View getPrimaryView() {
return viewGroupMixIn.getPrimaryView();
}

public void setPrimaryView(View v) {
public void setPrimaryView(@Nonnull View v) {
this.primaryView = v.getViewName();
}

@@ -30,3 +30,5 @@ StatusColumn.DisplayName=Status
WeatherColumn.DisplayName=Weather
DefaultViewsTabsBar.DisplayName=Default Views TabBar
DefaultMyViewsTabsBar.DisplayName=Default My Views TabBar

GlobalDefaultViewConfiguration.ViewDoesNotExist=The specified view does not exist: {0}
@@ -0,0 +1,66 @@
/*
* The MIT License
*
* Copyright (c) 2017 Oleg Nenashev.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package hudson.views;

import hudson.model.Descriptor;
import net.sf.json.JSONObject;
import static org.hamcrest.Matchers.*;
import org.junit.Assert;
import static org.junit.Assert.assertThat;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.kohsuke.stapler.MockStaplerRequestBuilder;
import org.kohsuke.stapler.StaplerRequest;

/**
* Tests of {@link GlobalDefaultViewConfiguration}.
* @author Oleg Nenashev
*/
public class GlobalDefaultViewConfigurationTest {

@Rule
public JenkinsRule j = new JenkinsRule();

@Test
@Issue("JENKINS-42717")
public void shouldNotFailIfTheDefaultViewIsMissing() {
String viewName = "NonExistentView";
GlobalDefaultViewConfiguration c = new GlobalDefaultViewConfiguration();

StaplerRequest create = new MockStaplerRequestBuilder(j, "/configure").build();
JSONObject params = new JSONObject();
params.accumulate("primaryView", viewName);
try {
c.configure(create, params);
} catch(Descriptor.FormException ex) {
assertThat("Wrong exception message for the form failure",
ex.getMessage(), containsString(Messages.GlobalDefaultViewConfiguration_ViewDoesNotExist(viewName)));
return;
}
Assert.fail("Expected FormException");
}

}
@@ -0,0 +1,81 @@
/*
* The MIT License
*
* Copyright (c) 2017 Oleg Nenashev.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.kohsuke.stapler;

import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import javax.annotation.Nonnull;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
import jenkins.model.Jenkins;
import org.eclipse.jetty.server.Request;
import org.jvnet.hudson.test.JenkinsRule;
import org.mockito.Mockito;
import org.springframework.web.multipart.support.DefaultMultipartHttpServletRequest;

/**
* Mocked version of {@link StaplerRequest}.
* @author Oleg Nenashev
*/
public class MockStaplerRequestBuilder{

private final JenkinsRule r;

private final List<AncestorImpl> ancestors = new ArrayList<>();
private final TokenList tokens;
final Map<String,Object> getters = new HashMap<>();
private Stapler stapler;

public MockStaplerRequestBuilder(@Nonnull JenkinsRule r, String url) {
this.r = r;
this.tokens = new TokenList(url);
}

public MockStaplerRequestBuilder withStapler(Stapler stapler) {
this.stapler = stapler;
return this;
}

public MockStaplerRequestBuilder withGetter(String objectName, Object object) {
this.getters.put(objectName, object);
return this;
}

public MockStaplerRequestBuilder withAncestor(AncestorImpl ancestor) {
this.ancestors.add(ancestor);
return this;
}

public StaplerRequest build() throws AssertionError {
HttpServletRequest rawRequest = Mockito.mock(HttpServletRequest.class);
return new RequestImpl(stapler != null ? stapler : new Stapler(), rawRequest, ancestors, tokens);
}

}

0 comments on commit 0e2cadf

Please sign in to comment.