Skip to content

Commit

Permalink
JENKINS-44579 avoid performance issues on delete by lazily removing f… (
Browse files Browse the repository at this point in the history
  • Loading branch information
James William Dumay committed Jun 1, 2017
1 parent 93d5be5 commit 34d5dec
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 20 deletions.
Expand Up @@ -5,7 +5,6 @@
import hudson.model.Item;
import hudson.model.User;
import hudson.model.listeners.ItemListener;
import hudson.plugins.favorite.FavoritePlugin;

import java.io.IOException;
import java.util.logging.Level;
Expand All @@ -32,20 +31,4 @@ public void onRenamed(Item item, String oldName, String newName) {
}
}
}

@Override
public void onDeleted(Item item) {
if (item instanceof AbstractProject<?, ?>) {
for (User user : User.getAll()) {
FavoriteUserProperty fup = user.getProperty(FavoriteUserProperty.class);
try {
if (fup != null) {
fup.deleteFavourite(item.getFullName());
}
} catch (IOException e) {
LOGGER.log(Level.WARNING, "Remove favourites deleted item <" + item.getFullName() + ">.", e);
}
}
}
}
}
Expand Up @@ -2,13 +2,14 @@

import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import hudson.Extension;
import hudson.model.Descriptor;
import hudson.model.UserProperty;
import hudson.model.UserPropertyDescriptor;
import hudson.plugins.favorite.assets.Asset;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.StaplerRequest;
Expand All @@ -18,10 +19,11 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.ConcurrentMap;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
* Do not use directly.
Expand All @@ -30,6 +32,8 @@
@ExportedBean(defaultVisibility = 999)
public class FavoriteUserProperty extends UserProperty {

private static final Logger LOGGER = Logger.getLogger(FavoriteUserProperty.class.getName());

@Extension
public static final UserPropertyDescriptor DESCRIPTOR = new FavoriteUserPropertyDescriptor();

Expand All @@ -41,12 +45,15 @@ public FavoriteUserProperty() {}

private ConcurrentMap<String, Boolean> data = Maps.newConcurrentMap();

private transient long lastValidated = 0;

/**
* Use {#getAllFavorites()}
* @return favorites
*/
@Deprecated
public List<String> getFavorites() {
removeFavoritesWhichDoNotExist();
return ImmutableList.copyOf(Maps.filterEntries(data, new Predicate<Entry<String, Boolean>>() {
@Override
public boolean apply(@Nullable Entry<String, Boolean> input) {
Expand All @@ -56,6 +63,7 @@ public boolean apply(@Nullable Entry<String, Boolean> input) {
}

public Set<String> getAllFavorites() {
removeFavoritesWhichDoNotExist();
return Maps.filterEntries(data, new Predicate<Entry<String, Boolean>>() {
@Override
public boolean apply(@Nullable Entry<String, Boolean> input) {
Expand Down Expand Up @@ -130,6 +138,19 @@ Object readResolve() {
return this;
}

private void removeFavoritesWhichDoNotExist() {
Jenkins jenkins = Jenkins.getInstance();
for (String fullName : ImmutableSet.copyOf(data.keySet())) {
if (jenkins.getItem(fullName) == null) {
try {
deleteFavourite(fullName);
} catch (IOException e) {
LOGGER.log(Level.SEVERE, "Could not purge favorite '" + fullName + "'", e);
}
}
}
}

public Class getAssetClass() {
return Asset.class;
}
Expand Down
@@ -1,16 +1,38 @@
package hudson.plugins.favorite.user;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import hudson.model.User;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;

import java.io.IOException;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

public class FavoriteUserPropertyTest {

@Rule
public JenkinsRule rule = new JenkinsRule();

User bob;

FavoriteUserProperty property;

@Before
public void createUserAndProperty() throws IOException {
bob = User.get("bob");
property = new FavoriteUserProperty();
bob.addProperty(property);
}

@Test
public void testMigration() throws Exception {
FavoriteUserProperty property = new FavoriteUserProperty();
property.favorites.add("foo");
property.favorites.add("bar");
property.favorites.add("baz");
Expand All @@ -27,4 +49,30 @@ public void testMigration() throws Exception {

assertEquals(ImmutableList.copyOf(property.getAllFavorites()), ImmutableList.copyOf(property.getFavorites()));
}

@Test
public void lazyDeletionGetAllFavorites() throws Exception {
rule.createFreeStyleProject("d");

property.addFavorite("a");
property.addFavorite("b");
property.addFavorite("c");
property.addFavorite("d");

// a,b,c do not exist and should be removed
assertEquals(Sets.newHashSet("d"), property.getAllFavorites());
}

@Test
public void lazyDeletionGetFavorites() throws Exception {
rule.createFreeStyleProject("d");

property.addFavorite("a");
property.addFavorite("b");
property.addFavorite("c");
property.addFavorite("d");

// a,b,c do not exist and should be removed
assertEquals(ImmutableSet.of("d"), ImmutableSet.copyOf(property.getFavorites()));
}
}

0 comments on commit 34d5dec

Please sign in to comment.