From 42b701ab1a257a673bb3d46487ec256317dc9547 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 18 Jun 2020 16:52:06 +0100 Subject: [PATCH] Make SingleSelectCapability thread-safe Closes gh-1105 --- .../test/InitializrMetadataTestBuilder.java | 8 +-- .../metadata/InitializrMetadata.java | 3 +- .../metadata/SingleSelectCapability.java | 62 ++++++++++++++++--- .../metadata/SingleSelectCapabilityTests.java | 16 ++--- 4 files changed, 66 insertions(+), 23 deletions(-) diff --git a/initializr-generator-test/src/main/java/io/spring/initializr/generator/test/InitializrMetadataTestBuilder.java b/initializr-generator-test/src/main/java/io/spring/initializr/generator/test/InitializrMetadataTestBuilder.java index 70fcc547..40f6ba0e 100644 --- a/initializr-generator-test/src/main/java/io/spring/initializr/generator/test/InitializrMetadataTestBuilder.java +++ b/initializr-generator-test/src/main/java/io/spring/initializr/generator/test/InitializrMetadataTestBuilder.java @@ -126,7 +126,7 @@ public class InitializrMetadataTestBuilder { packaging.setId(id); packaging.setName(id); packaging.setDefault(defaultValue); - it.getPackagings().getContent().add(packaging); + it.getPackagings().addContent(packaging); }); return this; } @@ -141,7 +141,7 @@ public class InitializrMetadataTestBuilder { element.setId(version); element.setName(version); element.setDefault(defaultValue); - it.getJavaVersions().getContent().add(element); + it.getJavaVersions().addContent(element); }); return this; } @@ -156,7 +156,7 @@ public class InitializrMetadataTestBuilder { element.setId(id); element.setName(id); element.setDefault(defaultValue); - it.getLanguages().getContent().add(element); + it.getLanguages().addContent(element); }); return this; } @@ -172,7 +172,7 @@ public class InitializrMetadataTestBuilder { element.setId(id); element.setName(id); element.setDefault(defaultValue); - it.getBootVersions().getContent().add(element); + it.getBootVersions().addContent(element); }); return this; } diff --git a/initializr-metadata/src/main/java/io/spring/initializr/metadata/InitializrMetadata.java b/initializr-metadata/src/main/java/io/spring/initializr/metadata/InitializrMetadata.java index 00cc86c6..345f0b11 100644 --- a/initializr-metadata/src/main/java/io/spring/initializr/metadata/InitializrMetadata.java +++ b/initializr-metadata/src/main/java/io/spring/initializr/metadata/InitializrMetadata.java @@ -200,8 +200,7 @@ public class InitializrMetadata { * @param versionsMetadata the Spring Boot boot versions metadata to use */ public void updateSpringBootVersions(List versionsMetadata) { - this.bootVersions.getContent().clear(); - this.bootVersions.getContent().addAll(versionsMetadata); + this.bootVersions.setContent(versionsMetadata); List bootVersions = this.bootVersions.getContent().stream().map((it) -> Version.parse(it.getId())) .collect(Collectors.toList()); VersionParser parser = new VersionParser(bootVersions); diff --git a/initializr-metadata/src/main/java/io/spring/initializr/metadata/SingleSelectCapability.java b/initializr-metadata/src/main/java/io/spring/initializr/metadata/SingleSelectCapability.java index 76c6d6d0..8b910c82 100644 --- a/initializr-metadata/src/main/java/io/spring/initializr/metadata/SingleSelectCapability.java +++ b/initializr-metadata/src/main/java/io/spring/initializr/metadata/SingleSelectCapability.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,8 +16,13 @@ package io.spring.initializr.metadata; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; -import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.Consumer; +import java.util.function.Function; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; @@ -30,7 +35,9 @@ import com.fasterxml.jackson.annotation.JsonProperty; public class SingleSelectCapability extends ServiceCapability> implements Defaultable { - private final List content = new CopyOnWriteArrayList<>(); + private final List content = new ArrayList<>(); + + private final ReadWriteLock contentLock = new ReentrantReadWriteLock(); @JsonCreator SingleSelectCapability(@JsonProperty("id") String id) { @@ -43,7 +50,18 @@ public class SingleSelectCapability extends ServiceCapability getContent() { - return this.content; + return Collections.unmodifiableList(withReadableContent(ArrayList::new)); + } + + public void addContent(DefaultMetadataElement element) { + withWritableContent((content) -> content.add(element)); + } + + public void setContent(List newContent) { + withWritableContent((content) -> { + content.clear(); + content.addAll(newContent); + }); } /** @@ -51,7 +69,8 @@ public class SingleSelectCapability extends ServiceCapability content.stream().filter(DefaultMetadataElement::isDefault).findFirst().orElse(null)); } /** @@ -60,16 +79,39 @@ public class SingleSelectCapability extends ServiceCapability id.equals(it.getId())).findFirst().orElse(null); + return withReadableContent( + (content) -> content.stream().filter((it) -> id.equals(it.getId())).findFirst().orElse(null)); } @Override public void merge(List otherContent) { - otherContent.forEach((it) -> { - if (get(it.getId()) == null) { - this.content.add(it); - } + withWritableContent((content) -> { + otherContent.forEach((it) -> { + if (get(it.getId()) == null) { + this.content.add(it); + } + }); }); } + private T withReadableContent(Function, T> consumer) { + this.contentLock.readLock().lock(); + try { + return consumer.apply(this.content); + } + finally { + this.contentLock.readLock().unlock(); + } + } + + private void withWritableContent(Consumer> consumer) { + this.contentLock.writeLock().lock(); + try { + consumer.accept(this.content); + } + finally { + this.contentLock.writeLock().unlock(); + } + } + } diff --git a/initializr-metadata/src/test/java/io/spring/initializr/metadata/SingleSelectCapabilityTests.java b/initializr-metadata/src/test/java/io/spring/initializr/metadata/SingleSelectCapabilityTests.java index ddb737b4..e00fa362 100755 --- a/initializr-metadata/src/test/java/io/spring/initializr/metadata/SingleSelectCapabilityTests.java +++ b/initializr-metadata/src/test/java/io/spring/initializr/metadata/SingleSelectCapabilityTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,8 @@ package io.spring.initializr.metadata; +import java.util.Arrays; + import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; @@ -34,17 +36,17 @@ class SingleSelectCapabilityTests { @Test void defaultNoDefault() { SingleSelectCapability capability = new SingleSelectCapability("test"); - capability.getContent().add(DefaultMetadataElement.create("foo", false)); - capability.getContent().add(DefaultMetadataElement.create("bar", false)); + capability.setContent(Arrays.asList(DefaultMetadataElement.create("foo", false), + DefaultMetadataElement.create("bar", false))); assertThat(capability.getDefault()).isNull(); } @Test void defaultType() { SingleSelectCapability capability = new SingleSelectCapability("test"); - capability.getContent().add(DefaultMetadataElement.create("foo", false)); + DefaultMetadataElement first = DefaultMetadataElement.create("foo", false); DefaultMetadataElement second = DefaultMetadataElement.create("bar", true); - capability.getContent().add(second); + capability.setContent(Arrays.asList(first, second)); assertThat(capability.getDefault()).isEqualTo(second); } @@ -52,11 +54,11 @@ class SingleSelectCapabilityTests { void mergeAddEntry() { SingleSelectCapability capability = new SingleSelectCapability("test"); DefaultMetadataElement foo = DefaultMetadataElement.create("foo", false); - capability.getContent().add(foo); + capability.setContent(Arrays.asList(foo)); SingleSelectCapability anotherCapability = new SingleSelectCapability("test"); DefaultMetadataElement bar = DefaultMetadataElement.create("bar", false); - anotherCapability.getContent().add(bar); + anotherCapability.setContent(Arrays.asList(bar)); capability.merge(anotherCapability); assertThat(capability.getContent()).hasSize(2);