Skip to content

Commit

Permalink
[JENKINS-20052] Refactor ListView getItems (#4466)
Browse files Browse the repository at this point in the history
* Refactor ListView getItems

Signed-off-by: Raihaan Shouhell <raihaan.shouhell@autodesk.com>

* Minor Refactor

* Optimization for specific case

* Migrate statusfilter to viewjobfilter

* Use unboxed boolean so we don't have to deal with null

* UI Changes

* Add tests and use pre-auth filters

* Add test with a simple filter

* Remove old tests as the new tests should cover this

* Change annotation

* Only iterate if there are names or a regex pattern
  • Loading branch information
res0nance committed May 29, 2020
1 parent eb80f74 commit 3fd66ff
Show file tree
Hide file tree
Showing 8 changed files with 290 additions and 127 deletions.
101 changes: 51 additions & 50 deletions core/src/main/java/hudson/model/ListView.java
Expand Up @@ -37,6 +37,7 @@
import hudson.util.FormValidation;
import hudson.util.HttpResponses;
import hudson.views.ListViewColumn;
import hudson.views.StatusFilter;
import hudson.views.ViewJobFilter;

import java.io.IOException;
Expand All @@ -53,6 +54,7 @@
import jenkins.model.ParameterizedJobMixIn;

import net.sf.json.JSONObject;
import org.acegisecurity.AccessDeniedException;
import org.jenkinsci.Symbol;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
Expand Down Expand Up @@ -101,8 +103,10 @@ public class ListView extends View implements DirectlyModifiableView {
/**
* Filter by enabled/disabled status of jobs.
* Null for no filter, true for enabled-only, false for disabled-only.
* Deprecated see {@link StatusFilter}
*/
private Boolean statusFilter;
@Deprecated
private transient Boolean statusFilter;

@DataBoundConstructor
public ListView(String name) {
Expand Down Expand Up @@ -145,6 +149,9 @@ private Object readResolve() {
}
initColumns();
initJobFilters();
if (statusFilter != null) {
jobFilters.add(0, new StatusFilter(statusFilter));
}
return this;
}

Expand Down Expand Up @@ -211,31 +218,47 @@ private List<TopLevelItem> getItems(boolean recurse) {
}

ItemGroup<? extends TopLevelItem> parent = getOwner().getItemGroup();
List<TopLevelItem> parentItems = new ArrayList<>(parent.getItems());
includeItems(parent, parentItems, names);

Boolean statusFilter = this.statusFilter; // capture the value to isolate us from concurrent update
Iterable<? extends TopLevelItem> candidates;
if (recurse) {
candidates = parent.getAllItems(TopLevelItem.class);
if (!names.isEmpty() || includePattern != null) {
items.addAll(parent.getAllItems(TopLevelItem.class, item -> {
String itemName = item.getRelativeNameFrom(parent);
if (names.contains(itemName)) {
return true;
}
if (includePattern != null) {
return includePattern.matcher(itemName).matches();
}
return false;
}));
}
} else {
candidates = parentItems;
}
for (TopLevelItem item : candidates) {
if (!names.contains(item.getRelativeNameFrom(getOwner().getItemGroup()))) continue;
// Add if no status filter or filter matches enabled/disabled status:
if(statusFilter == null || !(item instanceof ParameterizedJobMixIn.ParameterizedJob) // TODO or better to call the more generic Job.isBuildable?
|| ((ParameterizedJobMixIn.ParameterizedJob)item).isDisabled() ^ statusFilter)
items.add(item);
for (String name : names) {
try {
TopLevelItem i = parent.getItem(name);
if (i != null) {
items.add(i);
}
} catch (AccessDeniedException e) {
//Ignore
}
}
if (includePattern != null) {
items.addAll(parent.getItems(item -> {
String itemName = item.getRelativeNameFrom(parent);
return includePattern.matcher(itemName).matches();
}));
}
}

// check the filters
Iterable<ViewJobFilter> jobFilters = getJobFilters();
List<TopLevelItem> allItems = new ArrayList<>(parentItems);
if (recurse) allItems = expand(allItems, new ArrayList<>());
for (ViewJobFilter jobFilter: jobFilters) {
items = jobFilter.filter(items, allItems, this);
}
Collection<ViewJobFilter> jobFilters = getJobFilters();
if (!jobFilters.isEmpty()) {
List<TopLevelItem> candidates = recurse ? parent.getAllItems(TopLevelItem.class) : new ArrayList<>(parent.getItems());
// check the filters
for (ViewJobFilter jobFilter : jobFilters) {
items = jobFilter.filter(items, candidates, this);
}
}
// for sanity, trim off duplicates
items = new ArrayList<>(new LinkedHashSet<>(items));

Expand All @@ -251,39 +274,11 @@ public SearchIndexBuilder makeSearchIndex() {
return sib;
}

private List<TopLevelItem> expand(Collection<TopLevelItem> items, List<TopLevelItem> allItems) {
for (TopLevelItem item : items) {
if (item instanceof ItemGroup) {
ItemGroup<? extends Item> ig = (ItemGroup<? extends Item>) item;
expand(Util.filter(ig.getItems(), TopLevelItem.class), allItems);
}
allItems.add(item);
}
return allItems;
}

@Override
public boolean contains(TopLevelItem item) {
return getItems().contains(item);
}

private void includeItems(ItemGroup<? extends TopLevelItem> root, Collection<? extends Item> parentItems, SortedSet<String> names) {
if (includePattern != null) {
for (Item item : parentItems) {
if (recurse && item instanceof ItemGroup) {
ItemGroup<?> ig = (ItemGroup<?>) item;
includeItems(root, ig.getItems(), names);
}
if (item instanceof TopLevelItem) {
String itemName = item.getRelativeNameFrom(root);
if (includePattern.matcher(itemName).matches()) {
names.add(itemName);
}
}
}
}
}

public synchronized boolean jobNamesContains(TopLevelItem item) {
if (item == null) return false;
return jobNames.contains(item.getRelativeNameFrom(getOwner().getItemGroup()));
Expand Down Expand Up @@ -336,7 +331,9 @@ public void setRecurse(boolean recurse) {
/**
* Filter by enabled/disabled status of jobs.
* Null for no filter, true for enabled-only, false for disabled-only.
* Status filter is now controlled via a JobViewFilter, see {@link StatusFilter}
*/
@Deprecated
public Boolean getStatusFilter() {
return statusFilter;
}
Expand Down Expand Up @@ -488,6 +485,11 @@ public synchronized void setJobNames(Set<String> jobNames) {
this.jobNames = new TreeSet<>(jobNames);
}

/**
* Deprecated see, {@link StatusFilter}
* @param statusFilter
*/
@Deprecated
@DataBoundSetter
public void setStatusFilter(Boolean statusFilter) {
this.statusFilter = statusFilter;
Expand Down Expand Up @@ -613,5 +615,4 @@ private void deleteViewItem(Item item, ViewGroup vg, ListView lv) {
}
}
}

}
93 changes: 93 additions & 0 deletions core/src/main/java/hudson/views/StatusFilter.java
@@ -0,0 +1,93 @@
/*
* The MIT License
*
* Copyright (c) 20020, Autodesk, Inc., Raihaan Shouhell
*
* 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.Extension;
import hudson.model.Descriptor;
import hudson.model.TopLevelItem;
import hudson.model.View;
import jenkins.model.ParameterizedJobMixIn;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.DataBoundConstructor;

import edu.umd.cs.findbugs.annotations.NonNull;
import java.util.ArrayList;
import java.util.List;

/**
* Job Filter that will filter jobs based on its disabled status
*/
@Restricted(NoExternalUse.class)
public final class StatusFilter extends ViewJobFilter {

private final boolean statusFilter;

/**
* Creates a new status filter based on supplied boolean
* @param statusFilter true for enabled jobs only and false for disabled jobs only
*/
@DataBoundConstructor
public StatusFilter(boolean statusFilter) {
this.statusFilter = statusFilter;
}

/**
* {@inheritDoc}
*/
@Override
public List<TopLevelItem> filter(List<TopLevelItem> added, List<TopLevelItem> all, View filteringView) {
List<TopLevelItem> filtered = new ArrayList<>();
for (TopLevelItem item : added) {
if (!(item instanceof ParameterizedJobMixIn.ParameterizedJob) // TODO or better to call the more generic Job.isBuildable?
|| ((ParameterizedJobMixIn.ParameterizedJob<?,?>) item).isDisabled() ^ statusFilter)
filtered.add(item);
}
return filtered;
}

/**
* The setting of the job status filter
* @return true if only enabled jobs are shown and false if only disabled jobs are shown
*/
public boolean getStatusFilter() {
return statusFilter;
}

@Extension
public static class StatusFilterDescriptor extends Descriptor<ViewJobFilter> {
public StatusFilterDescriptor() {
super(StatusFilter.class);
}

/**
* {@inheritDoc}
*/
@NonNull
@Override
public String getDisplayName() {
return Messages.StatusFilter_DisplayName();
}
}
}
Expand Up @@ -29,14 +29,6 @@ THE SOFTWARE.

<f:section title="${%Job Filters}">

<f:entry title="${%Status Filter}" help="/help/view-config/statusFilter.html">
<select name="statusFilter" class="setting-input">
<f:option value="" selected="${it.statusFilter==null}">${%All selected jobs}</f:option>
<f:option value="1" selected="${it.statusFilter==true}">${%Enabled jobs only}</f:option>
<f:option value="2" selected="${it.statusFilter==false}">${%Disabled jobs only}</f:option>
</select>
</f:entry>

<f:entry field="recurse">
<f:checkbox title="${%Recurse in subfolders}" id="recurse" />
</f:entry>
Expand Down
1 change: 1 addition & 0 deletions core/src/main/resources/hudson/views/Messages.properties
Expand Up @@ -30,5 +30,6 @@ StatusColumn.DisplayName=Status
WeatherColumn.DisplayName=Weather
DefaultViewsTabsBar.DisplayName=Default Views TabBar
DefaultMyViewsTabsBar.DisplayName=Default My Views TabBar
StatusFilter.DisplayName=Status Filter

GlobalDefaultViewConfiguration.ViewDoesNotExist=The specified view does not exist: {0}
34 changes: 34 additions & 0 deletions core/src/main/resources/hudson/views/StatusFilter/config.jelly
@@ -0,0 +1,34 @@
<!--
The MIT License
Copyright (c) 2020, Autodesk, Inc., Raihaan Shouhell
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.
-->

<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define"
xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
<f:entry title="${%Status Filter}" help="/help/view-config/statusFilter.html">
<select name="statusFilter" class="setting-input">
<f:option value="1" selected="${it.statusFilter==true}">${%Enabled jobs only}</f:option>
<f:option value="2" selected="${it.statusFilter==false}">${%Disabled jobs only}</f:option>
</select>
</f:entry>
</j:jelly>
69 changes: 0 additions & 69 deletions core/src/test/java/hudson/model/ListViewTest.java

This file was deleted.

0 comments on commit 3fd66ff

Please sign in to comment.