handle pending plugin installations

This commit is contained in:
Sebastian Sdorra
2019-08-21 12:49:15 +02:00
parent 25cb0d6a25
commit 9514a94492
17 changed files with 292 additions and 66 deletions

View File

@@ -1,11 +1,19 @@
package sonia.scm.plugin;
import com.google.common.base.Preconditions;
public class AvailablePlugin implements Plugin {
private final AvailablePluginDescriptor pluginDescriptor;
private final boolean pending;
public AvailablePlugin(AvailablePluginDescriptor pluginDescriptor) {
this(pluginDescriptor, false);
}
private AvailablePlugin(AvailablePluginDescriptor pluginDescriptor, boolean pending) {
this.pluginDescriptor = pluginDescriptor;
this.pending = pending;
}
@Override
@@ -17,4 +25,13 @@ public class AvailablePlugin implements Plugin {
public PluginState getState() {
return PluginState.AVAILABLE;
}
public boolean isPending() {
return pending;
}
public AvailablePlugin install() {
Preconditions.checkState(!pending, "installation is already pending");
return new AvailablePlugin(pluginDescriptor, true);
}
}

View File

@@ -3,6 +3,14 @@ package sonia.scm.plugin;
public interface Plugin {
PluginDescriptor getDescriptor();
/**
* Returns plugin state.
*
* @deprecated State is now derived from concrete plugin implementations
* @return plugin state
*/
@Deprecated
PluginState getState();
}

View File

@@ -80,4 +80,9 @@ public interface PluginManager {
* @param restartAfterInstallation restart context after plugin installation
*/
void install(String name, boolean restartAfterInstallation);
/**
* Install all pending plugins and restart the scm context.
*/
void installPendingAndRestart();
}

View File

@@ -0,0 +1,32 @@
package sonia.scm.plugin;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
@ExtendWith(MockitoExtension.class)
class AvailablePluginTest {
@Mock
private AvailablePluginDescriptor descriptor;
@Test
void shouldReturnNewPendingPluginOnInstall() {
AvailablePlugin plugin = new AvailablePlugin(descriptor);
assertThat(plugin.isPending()).isFalse();
AvailablePlugin installed = plugin.install();
assertThat(installed.isPending()).isTrue();
}
@Test
void shouldThrowIllegalStateExceptionIfAlreadyPending() {
AvailablePlugin plugin = new AvailablePlugin(descriptor).install();
assertThrows(IllegalStateException.class, () -> plugin.install());
}
}

View File

@@ -10,7 +10,6 @@ import sonia.scm.plugin.PluginPermissions;
import sonia.scm.web.VndMediaType;
import javax.inject.Inject;
import javax.ws.rs.Consumes;
import javax.ws.rs.GET;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
@@ -74,7 +73,7 @@ public class AvailablePluginResource {
PluginPermissions.read().check();
Optional<AvailablePlugin> plugin = pluginManager.getAvailable(name);
if (plugin.isPresent()) {
return Response.ok(mapper.map(plugin.get())).build();
return Response.ok(mapper.mapAvailable(plugin.get())).build();
} else {
throw notFound(entity(InstalledPluginDescriptor.class, name));
}

View File

@@ -73,7 +73,7 @@ public class InstalledPluginResource {
PluginPermissions.read().check();
Optional<InstalledPlugin> pluginDto = pluginManager.getInstalled(name);
if (pluginDto.isPresent()) {
return Response.ok(mapper.map(pluginDto.get())).build();
return Response.ok(mapper.mapInstalled(pluginDto.get())).build();
} else {
throw notFound(entity(InstalledPluginDescriptor.class, name));
}

View File

@@ -11,6 +11,7 @@ import java.util.Set;
@Getter
@Setter
@NoArgsConstructor
@SuppressWarnings("squid:S2160") // we do not need equals for dto
public class PluginDto extends HalRepresentation {
private String name;
@@ -20,6 +21,7 @@ public class PluginDto extends HalRepresentation {
private String author;
private String category;
private String avatarUrl;
private boolean pending;
private Set<String> dependencies;
public PluginDto(Links links) {

View File

@@ -5,10 +5,8 @@ import de.otto.edison.hal.Embedded;
import de.otto.edison.hal.HalRepresentation;
import de.otto.edison.hal.Links;
import sonia.scm.plugin.AvailablePlugin;
import sonia.scm.plugin.PluginInformation;
import sonia.scm.plugin.InstalledPlugin;
import java.util.Collection;
import java.util.List;
import static de.otto.edison.hal.Embedded.embeddedBuilder;
@@ -27,12 +25,12 @@ public class PluginDtoCollectionMapper {
}
public HalRepresentation mapInstalled(List<InstalledPlugin> plugins) {
List<PluginDto> dtos = plugins.stream().map(mapper::map).collect(toList());
List<PluginDto> dtos = plugins.stream().map(mapper::mapInstalled).collect(toList());
return new HalRepresentation(createInstalledPluginsLinks(), embedDtos(dtos));
}
public HalRepresentation mapAvailable(List<AvailablePlugin> plugins) {
List<PluginDto> dtos = plugins.stream().map(mapper::map).collect(toList());
List<PluginDto> dtos = plugins.stream().map(mapper::mapAvailable).collect(toList());
return new HalRepresentation(createAvailablePluginsLinks(), embedDtos(dtos));
}

View File

@@ -3,9 +3,11 @@ package sonia.scm.api.v2.resources;
import de.otto.edison.hal.Links;
import org.mapstruct.Mapper;
import org.mapstruct.MappingTarget;
import sonia.scm.plugin.AvailablePlugin;
import sonia.scm.plugin.InstalledPlugin;
import sonia.scm.plugin.Plugin;
import sonia.scm.plugin.PluginInformation;
import sonia.scm.plugin.PluginState;
import sonia.scm.plugin.PluginPermissions;
import javax.inject.Inject;
@@ -20,34 +22,48 @@ public abstract class PluginDtoMapper {
public abstract void map(PluginInformation plugin, @MappingTarget PluginDto dto);
public PluginDto map(Plugin plugin) {
PluginDto dto = createDto(plugin);
public PluginDto mapInstalled(InstalledPlugin plugin) {
PluginDto dto = createDtoForInstalled(plugin);
map(dto, plugin);
return dto;
}
public PluginDto mapAvailable(AvailablePlugin plugin) {
PluginDto dto = createDtoForAvailable(plugin);
map(dto, plugin);
dto.setPending(plugin.isPending());
return dto;
}
private void map(PluginDto dto, Plugin plugin) {
dto.setDependencies(plugin.getDescriptor().getDependencies());
map(plugin.getDescriptor().getInformation(), dto);
if (dto.getCategory() == null) {
dto.setCategory("Miscellaneous");
}
return dto;
}
private PluginDto createDto(Plugin plugin) {
Links.Builder linksBuilder;
private PluginDto createDtoForAvailable(AvailablePlugin plugin) {
PluginInformation information = plugin.getDescriptor().getInformation();
PluginInformation pluginInformation = plugin.getDescriptor().getInformation();
Links.Builder links = linkingTo()
.self(resourceLinks.availablePlugin()
.self(information.getName()));
if (plugin.getState() != null && plugin.getState().equals(PluginState.AVAILABLE)) {
linksBuilder = linkingTo()
.self(resourceLinks.availablePlugin()
.self(pluginInformation.getName(), pluginInformation.getVersion()));
linksBuilder.single(link("install", resourceLinks.availablePlugin().install(pluginInformation.getName(), pluginInformation.getVersion())));
}
else {
linksBuilder = linkingTo()
.self(resourceLinks.installedPlugin()
.self(pluginInformation.getName()));
if (!plugin.isPending() && PluginPermissions.manage().isPermitted()) {
links.single(link("install", resourceLinks.availablePlugin().install(information.getName())));
}
return new PluginDto(linksBuilder.build());
return new PluginDto(links.build());
}
private PluginDto createDtoForInstalled(InstalledPlugin plugin) {
PluginInformation information = plugin.getDescriptor().getInformation();
Links.Builder links = linkingTo()
.self(resourceLinks.installedPlugin()
.self(information.getName()));
return new PluginDto(links.build());
}
}

View File

@@ -6,6 +6,7 @@ import javax.inject.Inject;
import java.net.URI;
import java.net.URISyntaxException;
@SuppressWarnings("squid:S1192") // string literals should not be duplicated
class ResourceLinks {
private final ScmPathInfoStore scmPathInfoStore;
@@ -694,12 +695,12 @@ class ResourceLinks {
availablePluginLinkBuilder = new LinkBuilder(pathInfo, PluginRootResource.class, AvailablePluginResource.class);
}
String self(String name, String version) {
return availablePluginLinkBuilder.method("availablePlugins").parameters().method("getAvailablePlugin").parameters(name, version).href();
String self(String name) {
return availablePluginLinkBuilder.method("availablePlugins").parameters().method("getAvailablePlugin").parameters(name).href();
}
String install(String name, String version) {
return availablePluginLinkBuilder.method("availablePlugins").parameters().method("installPlugin").parameters(name, version).href();
String install(String name) {
return availablePluginLinkBuilder.method("availablePlugins").parameters().method("installPlugin").parameters(name).href();
}
}

View File

@@ -67,6 +67,7 @@ public class DefaultPluginManager implements PluginManager {
private final PluginLoader loader;
private final PluginCenter center;
private final PluginInstaller installer;
private final List<PendingPluginInstallation> pendingQueue = new ArrayList<>();
@Inject
public DefaultPluginManager(ScmEventBus eventBus, PluginLoader loader, PluginCenter center, PluginInstaller installer) {
@@ -83,6 +84,15 @@ public class DefaultPluginManager implements PluginManager {
.stream()
.filter(filterByName(name))
.filter(this::isNotInstalled)
.map(p -> getPending(name).orElse(p))
.findFirst();
}
private Optional<AvailablePlugin> getPending(String name) {
return pendingQueue
.stream()
.map(PendingPluginInstallation::getPlugin)
.filter(filterByName(name))
.findFirst();
}
@@ -104,7 +114,11 @@ public class DefaultPluginManager implements PluginManager {
@Override
public List<AvailablePlugin> getAvailable() {
PluginPermissions.read().check();
return center.getAvailable().stream().filter(this::isNotInstalled).collect(Collectors.toList());
return center.getAvailable()
.stream()
.filter(this::isNotInstalled)
.map(p -> getPending(p.getDescriptor().getInformation().getName()).orElse(p))
.collect(Collectors.toList());
}
private <T extends Plugin> Predicate<T> filterByName(String name) {
@@ -129,11 +143,28 @@ public class DefaultPluginManager implements PluginManager {
throw ex;
}
}
if (restartAfterInstallation) {
eventBus.post(new RestartEvent(PluginManager.class, "plugin installation"));
if (!pendingInstallations.isEmpty()) {
if (restartAfterInstallation) {
restart("plugin installation");
} else {
pendingQueue.addAll(pendingInstallations);
}
}
}
@Override
public void installPendingAndRestart() {
PluginPermissions.manage().check();
if (!pendingQueue.isEmpty()) {
restart("install pending plugins");
}
}
private void restart(String cause) {
eventBus.post(new RestartEvent(PluginManager.class, cause));
}
private void cancelPending(List<PendingPluginInstallation> pendingInstallations) {
pendingInstallations.forEach(PendingPluginInstallation::cancel);
}
@@ -144,8 +175,12 @@ public class DefaultPluginManager implements PluginManager {
return plugins;
}
private boolean isInstalledOrPending(String name) {
return getInstalled(name).isPresent() || getPending(name).isPresent();
}
private void collectPluginsToInstall(List<AvailablePlugin> plugins, String name) {
if (!getInstalled(name).isPresent()) {
if (!isInstalledOrPending(name)) {
AvailablePlugin plugin = getAvailable(name).orElseThrow(() -> NotFoundException.notFound(entity(AvailablePlugin.class, name)));
Set<String> dependencies = plugin.getDescriptor().getDependencies();
@@ -157,7 +192,7 @@ public class DefaultPluginManager implements PluginManager {
plugins.add(plugin);
} else {
LOG.info("plugin {} is already installed, skipping installation", name);
LOG.info("plugin {} is already installed or installation is pending, skipping installation", name);
}
}
}

View File

@@ -33,7 +33,7 @@ class PluginInstaller {
Files.copy(input, file);
verifyChecksum(plugin, input.hash(), file);
return new PendingPluginInstallation(plugin, file);
return new PendingPluginInstallation(plugin.install(), file);
} catch (IOException ex) {
cleanup(file);
throw new PluginDownloadException("failed to download plugin", ex);

View File

@@ -117,7 +117,7 @@ class AvailablePluginResourceTest {
PluginDto pluginDto = new PluginDto();
pluginDto.setName("pluginName");
when(mapper.map(plugin)).thenReturn(pluginDto);
when(mapper.mapAvailable(plugin)).thenReturn(pluginDto);
MockHttpRequest request = MockHttpRequest.get("/v2/plugins/available/pluginName");
request.accept(VndMediaType.PLUGIN);

View File

@@ -110,7 +110,7 @@ class InstalledPluginResourceTest {
PluginDto pluginDto = new PluginDto();
pluginDto.setName("pluginName");
when(mapper.map(installedPlugin)).thenReturn(pluginDto);
when(mapper.mapInstalled(installedPlugin)).thenReturn(pluginDto);
MockHttpRequest request = MockHttpRequest.get("/v2/plugins/installed/pluginName");
request.accept(VndMediaType.PLUGIN);

View File

@@ -1,15 +1,20 @@
package sonia.scm.api.v2.resources;
import com.google.common.collect.ImmutableSet;
import org.apache.shiro.subject.Subject;
import org.apache.shiro.util.ThreadContext;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Answers;
import org.mockito.InjectMocks;
import org.mockito.Mockito;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import sonia.scm.plugin.Plugin;
import sonia.scm.plugin.PluginDescriptor;
import sonia.scm.plugin.AvailablePlugin;
import sonia.scm.plugin.AvailablePluginDescriptor;
import sonia.scm.plugin.InstalledPlugin;
import sonia.scm.plugin.PluginInformation;
import sonia.scm.plugin.PluginState;
import java.net.URI;
@@ -26,6 +31,19 @@ class PluginDtoMapperTest {
@InjectMocks
private PluginDtoMapperImpl mapper;
@Mock
private Subject subject;
@BeforeEach
void bindSubject() {
ThreadContext.bind(subject);
}
@AfterEach
void unbindSubject() {
ThreadContext.unbindSubject();
}
@Test
void shouldMapInformation() {
PluginInformation information = createPluginInformation();
@@ -54,27 +72,42 @@ class PluginDtoMapperTest {
@Test
void shouldAppendInstalledSelfLink() {
Plugin plugin = createPlugin(PluginState.INSTALLED);
InstalledPlugin plugin = createInstalled();
PluginDto dto = mapper.map(plugin);
PluginDto dto = mapper.mapInstalled(plugin);
assertThat(dto.getLinks().getLinkBy("self").get().getHref())
.isEqualTo("https://hitchhiker.com/v2/plugins/installed/scm-cas-plugin");
}
private InstalledPlugin createInstalled(PluginInformation information) {
InstalledPlugin plugin = mock(InstalledPlugin.class, Answers.RETURNS_DEEP_STUBS);
when(plugin.getDescriptor().getInformation()).thenReturn(information);
return plugin;
}
@Test
void shouldAppendAvailableSelfLink() {
Plugin plugin = createPlugin(PluginState.AVAILABLE);
AvailablePlugin plugin = createAvailable();
PluginDto dto = mapper.map(plugin);
PluginDto dto = mapper.mapAvailable(plugin);
assertThat(dto.getLinks().getLinkBy("self").get().getHref())
.isEqualTo("https://hitchhiker.com/v2/plugins/available/scm-cas-plugin");
}
@Test
void shouldAppendInstallLink() {
Plugin plugin = createPlugin(PluginState.AVAILABLE);
void shouldNotAppendInstallLinkWithoutPermissions() {
AvailablePlugin plugin = createAvailable();
PluginDto dto = mapper.map(plugin);
PluginDto dto = mapper.mapAvailable(plugin);
assertThat(dto.getLinks().getLinkBy("install")).isEmpty();
}
@Test
void shouldAppendInstallLink() {
when(subject.isPermitted("plugin:manage")).thenReturn(true);
AvailablePlugin plugin = createAvailable();
PluginDto dto = mapper.mapAvailable(plugin);
assertThat(dto.getLinks().getLinkBy("install").get().getHref())
.isEqualTo("https://hitchhiker.com/v2/plugins/available/scm-cas-plugin/install");
}
@@ -83,31 +116,32 @@ class PluginDtoMapperTest {
void shouldReturnMiscellaneousIfCategoryIsNull() {
PluginInformation information = createPluginInformation();
information.setCategory(null);
Plugin plugin = createPlugin(information, PluginState.AVAILABLE);
PluginDto dto = mapper.map(plugin);
AvailablePlugin plugin = createAvailable(information);
PluginDto dto = mapper.mapAvailable(plugin);
assertThat(dto.getCategory()).isEqualTo("Miscellaneous");
}
@Test
void shouldAppendDependencies() {
Plugin plugin = createPlugin(PluginState.AVAILABLE);
AvailablePlugin plugin = createAvailable();
when(plugin.getDescriptor().getDependencies()).thenReturn(ImmutableSet.of("one", "two"));
PluginDto dto = mapper.map(plugin);
PluginDto dto = mapper.mapAvailable(plugin);
assertThat(dto.getDependencies()).containsOnly("one", "two");
}
private Plugin createPlugin(PluginState state) {
return createPlugin(createPluginInformation(), state);
private InstalledPlugin createInstalled() {
return createInstalled(createPluginInformation());
}
private Plugin createPlugin(PluginInformation information, PluginState state) {
Plugin plugin = Mockito.mock(Plugin.class);
when(plugin.getState()).thenReturn(state);
PluginDescriptor descriptor = mock(PluginDescriptor.class);
private AvailablePlugin createAvailable() {
return createAvailable(createPluginInformation());
}
private AvailablePlugin createAvailable(PluginInformation information) {
AvailablePluginDescriptor descriptor = mock(AvailablePluginDescriptor.class);
when(descriptor.getInformation()).thenReturn(information);
when(plugin.getDescriptor()).thenReturn(descriptor);
return plugin;
return new AvailablePlugin(descriptor);
}
}

View File

@@ -11,6 +11,7 @@ import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Answers;
import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
@@ -22,6 +23,7 @@ import java.util.List;
import java.util.Optional;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.in;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.*;
@@ -46,6 +48,14 @@ class DefaultPluginManagerTest {
@Mock
private Subject subject;
@BeforeEach
void mockInstaller() {
lenient().when(installer.install(any())).then(ic -> {
AvailablePlugin plugin = ic.getArgument(0);
return new PendingPluginInstallation(plugin.install(), null);
});
}
@Nested
class WithAdminPermissions {
@@ -180,7 +190,10 @@ class DefaultPluginManagerTest {
manager.install("scm-review-plugin", false);
verify(installer).install(review);
ArgumentCaptor<AvailablePlugin> captor = ArgumentCaptor.forClass(AvailablePlugin.class);
verify(installer).install(captor.capture());
assertThat(captor.getValue().getDescriptor().getInformation().getName()).isEqualTo("scm-review-plugin");
}
@Test
@@ -230,6 +243,66 @@ class DefaultPluginManagerTest {
verify(eventBus).post(any(RestartEvent.class));
}
@Test
void shouldNotSendRestartEventIfNoPluginWasInstalled() {
InstalledPlugin gitInstalled = createInstalled("scm-git-plugin");
when(loader.getInstalledPlugins()).thenReturn(ImmutableList.of(gitInstalled));
manager.install("scm-git-plugin", true);
verify(eventBus, never()).post(any());
}
@Test
void shouldNotInstallAlreadyPendingPlugins() {
AvailablePlugin review = createAvailable("scm-review-plugin");
when(center.getAvailable()).thenReturn(ImmutableSet.of(review));
manager.install("scm-review-plugin", false);
manager.install("scm-review-plugin", false);
// only one interaction
verify(installer).install(any());
}
@Test
void shouldSendRestartEvent() {
AvailablePlugin review = createAvailable("scm-review-plugin");
when(center.getAvailable()).thenReturn(ImmutableSet.of(review));
manager.install("scm-review-plugin", false);
manager.installPendingAndRestart();
verify(eventBus).post(any(RestartEvent.class));
}
@Test
void shouldNotSendRestartEventWithoutPendingPlugins() {
manager.installPendingAndRestart();
verify(eventBus, never()).post(any());
}
@Test
void shouldReturnSingleAvailableAsPending() {
AvailablePlugin review = createAvailable("scm-review-plugin");
when(center.getAvailable()).thenReturn(ImmutableSet.of(review));
manager.install("scm-review-plugin", false);
Optional<AvailablePlugin> available = manager.getAvailable("scm-review-plugin");
assertThat(available.get().isPending()).isTrue();
}
@Test
void shouldReturnAvailableAsPending() {
AvailablePlugin review = createAvailable("scm-review-plugin");
when(center.getAvailable()).thenReturn(ImmutableSet.of(review));
manager.install("scm-review-plugin", false);
List<AvailablePlugin> available = manager.getAvailable();
assertThat(available.get(0).isPending()).isTrue();
}
}
@Nested
@@ -275,6 +348,11 @@ class DefaultPluginManagerTest {
assertThrows(AuthorizationException.class, () -> manager.install("test", false));
}
@Test
void shouldThrowAuthorizationExceptionsForInstallPendingAndRestart() {
assertThrows(AuthorizationException.class, () -> manager.installPendingAndRestart());
}
}
private AvailablePlugin createAvailable(String name) {
@@ -296,9 +374,9 @@ class DefaultPluginManagerTest {
}
private AvailablePlugin createAvailable(PluginInformation information) {
AvailablePlugin plugin = mock(AvailablePlugin.class, Answers.RETURNS_DEEP_STUBS);
returnInformation(plugin, information);
return plugin;
AvailablePluginDescriptor descriptor = mock(AvailablePluginDescriptor.class);
lenient().when(descriptor.getInformation()).thenReturn(information);
return new AvailablePlugin(descriptor);
}
private void returnInformation(Plugin mockedPlugin, PluginInformation information) {

View File

@@ -63,7 +63,8 @@ class PluginInstallerTest {
PendingPluginInstallation pending = installer.install(gitPlugin);
assertThat(pending).isNotNull();
assertThat(pending.getPlugin()).isSameAs(gitPlugin);
assertThat(pending.getPlugin().getDescriptor()).isEqualTo(gitPlugin.getDescriptor());
assertThat(pending.getPlugin().isPending()).isTrue();
}
private void mockContent(String content) throws IOException {