Skip to content
Permalink
Browse files

[JENKINS-42645] Case insensitive search by default for new and anonym…

…ous users (#2801)

* [JENKINS-42645] Case insensitive search by default

* [JENKINS-42960] Search in FixedSet more locale friendly

String.equalsIgnoreCase is safer than toLowerCase when non English
locales are used.

* [JENKINS-42645] Review remarks
  • Loading branch information...
szpak authored and oleg-nenashev committed Apr 7, 2017
1 parent 25ef87d commit b831acd9854b525d680ca72fd218c848121b9d3f
@@ -27,12 +27,15 @@
import java.util.Collection;
import java.util.List;

import org.apache.commons.lang.StringUtils;

/**
* Set of {@link SearchItem}s that are statically known upfront.
*
* @author Kohsuke Kawaguchi
*/
public class FixedSet implements SearchIndex {

private final Collection<? extends SearchItem> items;

public FixedSet(Collection<? extends SearchItem> items) {
@@ -45,27 +48,21 @@ public FixedSet(SearchItem... items) {

public void find(String token, List<SearchItem> result) {
boolean caseInsensitive = UserSearchProperty.isCaseInsensitive();
for (SearchItem i : items){
for (SearchItem i : items) {
String name = i.getSearchName();
if(caseInsensitive){
token=token.toLowerCase();
name=name.toLowerCase();
}
if(token.equals(i.getSearchName()))
if (name.equals(token) || (caseInsensitive && name.equalsIgnoreCase(token))) {
result.add(i);
}
}
}

public void suggest(String token, List<SearchItem> result) {
boolean caseInsensitive = UserSearchProperty.isCaseInsensitive();
for (SearchItem i : items){
for (SearchItem i : items) {
String name = i.getSearchName();
if(caseInsensitive){
token=token.toLowerCase();
name=name.toLowerCase();
}
if(name.contains(token))
if (name.contains(token) || (caseInsensitive && StringUtils.containsIgnoreCase(name, token))) {
result.add(i);
}
}
}
}
@@ -11,7 +11,9 @@
import org.kohsuke.stapler.export.Exported;

public class UserSearchProperty extends hudson.model.UserProperty {


private static final boolean DEFAULT_SEARCH_CASE_INSENSITIVE_MODE = true;

private final boolean insensitiveSearch;

public UserSearchProperty(boolean insensitiveSearch) {
@@ -25,11 +27,12 @@ public boolean getInsensitiveSearch() {

public static boolean isCaseInsensitive(){
User user = User.current();
boolean caseInsensitive = false;
if(user!=null && user.getProperty(UserSearchProperty.class).getInsensitiveSearch()){//Searching for anonymous user is case-sensitive
caseInsensitive=true;

if (user == null) {
return DEFAULT_SEARCH_CASE_INSENSITIVE_MODE;
}
return caseInsensitive;

return user.getProperty(UserSearchProperty.class).getInsensitiveSearch();
}


@@ -40,7 +43,7 @@ public String getDisplayName() {
}

public UserProperty newInstance(User user) {
return new UserSearchProperty(false); //default setting is case-sensitive searching
return new UserSearchProperty(DEFAULT_SEARCH_CASE_INSENSITIVE_MODE);
}

@Override
@@ -13,6 +13,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;

import javax.servlet.ServletException;
@@ -98,7 +99,7 @@ public void getAllItems() throws Exception {
final TopLevelItem rightJob = createJob("rightJob");

Mockito.when(leftView.getItems()).thenReturn(Arrays.asList(leftJob, sharedJob));
Mockito.when(rightView.getItems()).thenReturn(Arrays.asList(rightJob));
Mockito.when(rightView.getItems()).thenReturn(Collections.singletonList(rightJob));

final TopLevelItem[] expected = new TopLevelItem[] {rootJob, sharedJob, leftJob, rightJob};

@@ -37,8 +37,8 @@
import hudson.model.StringParameterValue;

import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.mockito.Mockito;

import java.io.IOException;
@@ -326,40 +326,39 @@ public void test_search_runs_by_build_number() throws IOException {
}

@Test
public void test_search_should_be_case_sensitive_for_anonymous_user() throws IOException {
//given
HistoryPageFilter<ModelObject> historyPageFilter = newPage(5, null, null);
//and
historyPageFilter.setSearchString("failure");
//and
@Issue("JENKINS-42645")
public void should_be_case_insensitive_by_default() throws IOException {
List<ModelObject> runs = Lists.<ModelObject>newArrayList(new MockRun(2, Result.FAILURE), new MockRun(1, Result.SUCCESS));
List<Queue.Item> queueItems = newQueueItems(3, 4);

//when
historyPageFilter.add(runs, queueItems);
assertOneMatchingBuildForGivenSearchStringAndRunItems("failure", runs);
}

//then
Assert.assertEquals(0, historyPageFilter.runs.size());
@Test
public void should_lower_case_search_string_in_case_insensitive_search() throws IOException {
List<ModelObject> runs = Lists.<ModelObject>newArrayList(new MockRun(2, Result.FAILURE), new MockRun(1, Result.SUCCESS));
assertOneMatchingBuildForGivenSearchStringAndRunItems("FAILure", runs);
}

@Test
public void test_search_builds_by_build_variables() throws IOException {
@Issue("JENKINS-40718")
public void should_search_builds_by_build_variables() throws IOException {
List<ModelObject> runs = ImmutableList.<ModelObject>of(
new MockBuild(2).withBuildVariables(ImmutableMap.of("env", "dummyEnv")),
new MockBuild(1).withBuildVariables(ImmutableMap.of("env", "otherEnv")));
assertOneMatchingBuildForGivenSearchStringAndRunItems("dummyEnv", runs);
}

@Test
public void test_search_builds_by_build_params() throws IOException {
@Issue("JENKINS-40718")
public void should_search_builds_by_build_params() throws IOException {
List<ModelObject> runs = ImmutableList.<ModelObject>of(
new MockBuild(2).withBuildParameters(ImmutableMap.of("env", "dummyEnv")),
new MockBuild(1).withBuildParameters(ImmutableMap.of("env", "otherEnv")));
assertOneMatchingBuildForGivenSearchStringAndRunItems("dummyEnv", runs);
}

@Test
public void test_ignore_sensitive_parameters_in_search_builds_by_build_params() throws IOException {
@Issue("JENKINS-40718")
public void should_ignore_sensitive_parameters_in_search_builds_by_build_params() throws IOException {
List<ModelObject> runs = ImmutableList.<ModelObject>of(
new MockBuild(2).withBuildParameters(ImmutableMap.of("plainPassword", "pass1plain")),
new MockBuild(1).withSensitiveBuildParameters("password", "pass1"));
@@ -8,6 +8,7 @@
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.mockito.Mockito;

@@ -25,40 +26,57 @@

/**
* TODO: Code partially duplicated with HistoryPageFilterTest in core
*
* Search in case insensitive mode is tested by unit tests in HistoryPageFilterTest.
*/
public class HistoryPageFilterInsensitiveSearchTest {
@Issue({"JENKINS-40718", "JENKINS-42645"})
public class HistoryPageFilterCaseSensitiveSearchTest {

private static final String TEST_USER_NAME = "testUser";

@Rule
public JenkinsRule j = new JenkinsRule();

@Test
public void should_search_insensitively_when_enabled_for_user() throws IOException {
setUserContextAndAssertCaseInsensitivitySearchForGivenSearchString("failure");
public void should_search_case_sensitively_when_enabled_for_user() throws IOException {
setCaseSensitiveSearchForUserAndCheckAssertionForGivenSearchString("FAILURE", new SearchResultAssertFunction() {
@Override
public void doAssertion(HistoryPageFilter<ModelObject> historyPageFilter) {
Assert.assertEquals(1, historyPageFilter.runs.size());
Assert.assertEquals(HistoryPageEntry.getEntryId(2), historyPageFilter.runs.get(0).getEntryId());
}
});
}

@Test
public void should_also_lower_search_query_in_insensitive_search_enabled() throws IOException {
setUserContextAndAssertCaseInsensitivitySearchForGivenSearchString("FAILure");
public void should_skip_result_with_different_capitalization_when_case_sensitively_search_is_enabled_for_user() throws IOException {
setCaseSensitiveSearchForUserAndCheckAssertionForGivenSearchString("failure", new SearchResultAssertFunction() {
@Override
public void doAssertion(HistoryPageFilter<ModelObject> historyPageFilter) {
Assert.assertEquals(0, historyPageFilter.runs.size());
}
});
}

private void setUserContextAndAssertCaseInsensitivitySearchForGivenSearchString(final String searchString) throws IOException {
private void setCaseSensitiveSearchForUserAndCheckAssertionForGivenSearchString(final String searchString,
SearchResultAssertFunction assertionOnSearchResults) throws IOException {
AuthorizationStrategy.Unsecured strategy = new AuthorizationStrategy.Unsecured();
j.jenkins.setAuthorizationStrategy(strategy);
j.jenkins.setSecurityRealm(j.createDummySecurityRealm());

UsernamePasswordAuthenticationToken testUserAuthentication = new UsernamePasswordAuthenticationToken(TEST_USER_NAME, "any");
try (ACLContext acl = ACL.as(testUserAuthentication)) {
User.get(TEST_USER_NAME).addProperty(new UserSearchProperty(true));
try (ACLContext ignored = ACL.as(testUserAuthentication)) {
User.get(TEST_USER_NAME).addProperty(new UserSearchProperty(false));

//test logic
List<ModelObject> runs = ImmutableList.<ModelObject>of(new MockRun(2, Result.FAILURE), new MockRun(1, Result.SUCCESS));
assertOneMatchingBuildForGivenSearchStringAndRunItems(searchString, runs);
final List<ModelObject> runs = ImmutableList.<ModelObject>of(new MockRun(2, Result.FAILURE), new MockRun(1, Result.SUCCESS));
assertNoMatchingBuildsForGivenSearchStringAndRunItems(searchString, runs, assertionOnSearchResults);
}

}

private void assertOneMatchingBuildForGivenSearchStringAndRunItems(String searchString, List<ModelObject> runs) {
private void assertNoMatchingBuildsForGivenSearchStringAndRunItems(String searchString, List<ModelObject> runs,
SearchResultAssertFunction assertionOnSearchResults) {
//given
HistoryPageFilter<ModelObject> historyPageFilter = new HistoryPageFilter<>(5);
//and
@@ -68,8 +86,7 @@ private void assertOneMatchingBuildForGivenSearchStringAndRunItems(String search
historyPageFilter.add(runs, Collections.<Queue.Item>emptyList());

//then
Assert.assertEquals(1, historyPageFilter.runs.size());
Assert.assertEquals(HistoryPageEntry.getEntryId(2), historyPageFilter.runs.get(0).getEntryId());
assertionOnSearchResults.doAssertion(historyPageFilter);
}

@SuppressWarnings("unchecked")
@@ -111,4 +128,9 @@ public int getNumber() {
return (int) queueId;
}
}

//Waiting for Java 8... - coming soon - April 2017?
private interface SearchResultAssertFunction {
void doAssertion(HistoryPageFilter<ModelObject> historyPageFilter);
}
}

0 comments on commit b831acd

Please sign in to comment.
You can’t perform that action at this time.