From 288b6bb7ec683585769acb7c062ba6591e1980a8 Mon Sep 17 00:00:00 2001 From: John Blum Date: Mon, 8 Jun 2020 15:16:16 -0700 Subject: [PATCH] Fix bug in cache data export caused by Apache Geode's JVM Shutdown Hook. Resolves gh-88. --- .../DataImportExportAutoConfiguration.java | 69 ++++++- .../main/resources/META-INF/spring.factories | 1 + .../java/example/app/golf/model/Golfer.java | 58 ++++++ ...portAutoConfigurationIntegrationTests.java | 178 ++++++++++++++++++ 4 files changed, 301 insertions(+), 5 deletions(-) create mode 100644 spring-geode-autoconfigure/src/test/java/example/app/golf/model/Golfer.java create mode 100644 spring-geode-autoconfigure/src/test/java/org/springframework/geode/boot/autoconfigure/data/CacheDataExportAutoConfigurationIntegrationTests.java diff --git a/spring-geode-autoconfigure/src/main/java/org/springframework/geode/boot/autoconfigure/DataImportExportAutoConfiguration.java b/spring-geode-autoconfigure/src/main/java/org/springframework/geode/boot/autoconfigure/DataImportExportAutoConfiguration.java index 410a2a80..750a38ad 100644 --- a/spring-geode-autoconfigure/src/main/java/org/springframework/geode/boot/autoconfigure/DataImportExportAutoConfiguration.java +++ b/spring-geode-autoconfigure/src/main/java/org/springframework/geode/boot/autoconfigure/DataImportExportAutoConfiguration.java @@ -16,26 +16,32 @@ package org.springframework.geode.boot.autoconfigure; import java.util.Optional; +import java.util.function.Predicate; import org.apache.geode.cache.GemFireCache; +import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.condition.AnyNestedCondition; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.env.EnvironmentPostProcessor; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Condition; import org.springframework.context.annotation.ConditionContext; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.Environment; import org.springframework.core.type.AnnotatedTypeMetadata; import org.springframework.data.gemfire.CacheFactoryBean; import org.springframework.geode.boot.autoconfigure.support.PdxInstanceWrapperRegionAspect; import org.springframework.geode.cache.SimpleCacheResolver; +import org.springframework.geode.data.AbstractCacheDataImporterExporter; import org.springframework.geode.data.json.JsonCacheDataImporterExporter; import org.springframework.lang.NonNull; +import org.springframework.lang.Nullable; /** * Spring Boot {@link EnableAutoConfiguration auto-configuration} for cache data import/export. @@ -49,12 +55,13 @@ import org.springframework.lang.NonNull; * @see org.springframework.boot.autoconfigure.condition.ConditionalOnProperty * @see org.springframework.context.annotation.Bean * @see org.springframework.context.annotation.Condition + * @see org.springframework.context.annotation.ConditionContext * @see org.springframework.context.annotation.Conditional * @see org.springframework.context.annotation.Configuration * @see org.springframework.core.env.Environment * @see org.springframework.core.type.AnnotatedTypeMetadata * @see org.springframework.data.gemfire.CacheFactoryBean - * @see PdxInstanceWrapperRegionAspect + * @see org.springframework.geode.boot.autoconfigure.support.PdxInstanceWrapperRegionAspect * @see org.springframework.geode.cache.SimpleCacheResolver * @see org.springframework.geode.data.json.JsonCacheDataImporterExporter * @since 1.3.0 @@ -65,6 +72,7 @@ import org.springframework.lang.NonNull; @SuppressWarnings("unused") public class DataImportExportAutoConfiguration { + protected static final String GEMFIRE_DISABLE_SHUTDOWN_HOOK = "gemfire.disableShutdownHook"; protected static final String PDX_READ_SERIALIZED_PROPERTY = "spring.data.gemfire.pdx.read-serialized"; protected static final String REGION_ADVICE_ENABLED_PROPERTY = "spring.boot.data.gemfire.cache.region.advice.enabled"; @@ -90,7 +98,7 @@ public class DataImportExportAutoConfiguration { static class AdviseRegionOnRegionAdviceEnabledProperty { } @Conditional(PdxReadSerializedCondition.class) - static class AdviceRegionOnPdxReadSerializedCondition { } + static class AdviseRegionOnPdxReadSerializedCondition { } } @@ -98,21 +106,72 @@ public class DataImportExportAutoConfiguration { @Override public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { - return isPdxReadSerializedTrue(context.getEnvironment()) || isCachePdxReadSerializedTrue(); + return isPdxReadSerializedEnabled(context.getEnvironment()) || isCachePdxReadSerializedEnabled(); } - private boolean isCachePdxReadSerializedTrue() { + private boolean isCachePdxReadSerializedEnabled() { return SimpleCacheResolver.getInstance().resolve() .filter(GemFireCache::getPdxReadSerialized) .isPresent(); } - private boolean isPdxReadSerializedTrue(@NonNull Environment environment) { + private boolean isPdxReadSerializedEnabled(@NonNull Environment environment) { return Optional.ofNullable(environment) .filter(env -> env.getProperty(PDX_READ_SERIALIZED_PROPERTY, Boolean.class, false)) .isPresent(); } } + + private static final boolean DEFAULT_EXPORT_ENABLED = false; + + private static final Predicate disableGemFireShutdownHookPredicate = environment -> + Optional.ofNullable(environment) + .filter(env -> env.getProperty(CacheDataImporterExporterReference.EXPORT_ENABLED_PROPERTY_NAME, + Boolean.class, DEFAULT_EXPORT_ENABLED)) + .isPresent(); + + static abstract class AbstractDisableGemFireShutdownHookSupport { + + boolean shouldDisableGemFireShutdownHook(@Nullable Environment environment) { + return disableGemFireShutdownHookPredicate.test(environment); + } + + /** + * If we do not disable GemFire/Geode's {@link org.apache.geode.distributed.DistributedSystem} JRE/JVM runtime + * shutdown hook then the {@link org.apache.geode.cache.Region} is prematurely closed by the JRE/JVM shutdown hook + * before Spring's {@link org.springframework.beans.factory.config.DestructionAwareBeanPostProcessor}s can do their + * work of exporting data from the {@link org.apache.geode.cache.Region} as JSON. + */ + void disableGemFireShutdownHook(@Nullable Environment environment) { + System.setProperty(GEMFIRE_DISABLE_SHUTDOWN_HOOK, Boolean.TRUE.toString()); + } + } + + static abstract class CacheDataImporterExporterReference extends AbstractCacheDataImporterExporter { + static final String EXPORT_ENABLED_PROPERTY_NAME = + AbstractCacheDataImporterExporter.CACHE_DATA_EXPORT_ENABLED_PROPERTY_NAME; + } + + static class DisableGemFireShutdownHookCondition extends AbstractDisableGemFireShutdownHookSupport + implements Condition { + + @Override + public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { + return shouldDisableGemFireShutdownHook(context.getEnvironment()); + } + } + + public static class DisableGemFireShutdownHookEnvironmentPostProcessor + extends AbstractDisableGemFireShutdownHookSupport implements EnvironmentPostProcessor { + + @Override + public void postProcessEnvironment(ConfigurableEnvironment environment, SpringApplication application) { + + if (shouldDisableGemFireShutdownHook(environment)) { + disableGemFireShutdownHook(environment); + } + } + } } diff --git a/spring-geode-autoconfigure/src/main/resources/META-INF/spring.factories b/spring-geode-autoconfigure/src/main/resources/META-INF/spring.factories index 20a16639..65cfea38 100644 --- a/spring-geode-autoconfigure/src/main/resources/META-INF/spring.factories +++ b/spring-geode-autoconfigure/src/main/resources/META-INF/spring.factories @@ -21,5 +21,6 @@ org.springframework.geode.boot.autoconfigure.SslAutoConfiguration # Environment Post Processing org.springframework.boot.env.EnvironmentPostProcessor=\ org.springframework.geode.boot.autoconfigure.ClientSecurityAutoConfiguration.AutoConfiguredCloudSecurityEnvironmentPostProcessor,\ +org.springframework.geode.boot.autoconfigure.DataImportExportAutoConfiguration.DisableGemFireShutdownHookEnvironmentPostProcessor,\ org.springframework.geode.boot.autoconfigure.SpringSessionAutoConfiguration.SpringSessionPropertiesEnvironmentPostProcessor,\ org.springframework.geode.boot.autoconfigure.SslAutoConfiguration.SslEnvironmentPostProcessor diff --git a/spring-geode-autoconfigure/src/test/java/example/app/golf/model/Golfer.java b/spring-geode-autoconfigure/src/test/java/example/app/golf/model/Golfer.java new file mode 100644 index 00000000..6b8af52f --- /dev/null +++ b/spring-geode-autoconfigure/src/test/java/example/app/golf/model/Golfer.java @@ -0,0 +1,58 @@ +/* + * Copyright 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ +package example.app.golf.model; + +import org.springframework.data.annotation.Id; +import org.springframework.data.gemfire.mapping.annotation.Region; + +import lombok.AccessLevel; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.NonNull; +import lombok.RequiredArgsConstructor; + +/** + * An Abstract Data Type (ADT) that models a person who golfs. + * + * @author John Blum + * @since 1.3.0 + */ +@Region("Golfers") +@Getter +@EqualsAndHashCode(of = "name") +@NoArgsConstructor(access = AccessLevel.PROTECTED) +@RequiredArgsConstructor(staticName = "newGolfer") +public class Golfer { + + @Id @NonNull + private Long id; + + @NonNull + private String name; + + private Integer handicap; + + public Golfer withHandicap(int handicap) { + this.handicap = handicap; + return this; + } + + @Override + public String toString() { + return String.format("%s:%d", getName(), getHandicap()); + } +} diff --git a/spring-geode-autoconfigure/src/test/java/org/springframework/geode/boot/autoconfigure/data/CacheDataExportAutoConfigurationIntegrationTests.java b/spring-geode-autoconfigure/src/test/java/org/springframework/geode/boot/autoconfigure/data/CacheDataExportAutoConfigurationIntegrationTests.java new file mode 100644 index 00000000..1b6b4241 --- /dev/null +++ b/spring-geode-autoconfigure/src/test/java/org/springframework/geode/boot/autoconfigure/data/CacheDataExportAutoConfigurationIntegrationTests.java @@ -0,0 +1,178 @@ +/* + * Copyright 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ +package org.springframework.geode.boot.autoconfigure.data; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.File; +import java.io.IOException; +import java.time.Duration; +import java.util.HashSet; +import java.util.Set; +import java.util.stream.StreamSupport; + +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.ObjectMapper; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import org.apache.geode.cache.Region; + +import org.springframework.boot.ApplicationRunner; +import org.springframework.boot.WebApplicationType; +import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.boot.builder.SpringApplicationBuilder; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Profile; +import org.springframework.data.gemfire.GemfireTemplate; +import org.springframework.data.gemfire.config.annotation.EnableEntityDefinedRegions; +import org.springframework.data.gemfire.tests.integration.ForkingClientServerIntegrationTestsSupport; +import org.springframework.data.gemfire.tests.process.ProcessWrapper; +import org.springframework.data.gemfire.tests.util.FileSystemUtils; +import org.springframework.data.gemfire.tests.util.FileUtils; +import org.springframework.geode.boot.autoconfigure.DataImportExportAutoConfiguration; +import org.springframework.geode.config.annotation.EnableClusterAware; + +import example.app.golf.model.Golfer; + +/** + * Integration Tests for {@link DataImportExportAutoConfiguration}, which specifically tests the export of + * {@link Region} values (data) to JSON on Spring Boot application (JVM) shutdown. + * + * @author John Blum + * @see org.junit.Test + * @see org.apache.geode.cache.Region + * @see org.springframework.boot.SpringApplication + * @see org.springframework.boot.autoconfigure.SpringBootApplication + * @see org.springframework.context.annotation.Profile + * @see org.springframework.geode.boot.autoconfigure.DataImportExportAutoConfiguration + * @since 1.3.0 + */ +public class CacheDataExportAutoConfigurationIntegrationTests extends ForkingClientServerIntegrationTestsSupport { + + private static final File GEODE_WORKING_DIRECTORY = + new File(String.format("cache-data-export-%d", System.currentTimeMillis())); + + private static ProcessWrapper process; + + private static final String DATA_GOLFERS_JSON = "data-golfers.json"; + + @BeforeClass + public static void runGeodeProcess() throws IOException { + + System.setProperty(DIRECTORY_DELETE_ON_EXIT_PROPERTY, Boolean.FALSE.toString()); + + process = run(GEODE_WORKING_DIRECTORY, TestGeodeConfiguration.class, + "-Dspring.profiles.active=EXPORT", "-Dspring.boot.data.gemfire.cache.data.export.enabled=true"); + + assertThat(process).isNotNull(); + + waitOn(() -> !process.isRunning(), Duration.ofSeconds(20).toMillis(), Duration.ofSeconds(2).toMillis()); + } + + @AfterClass + public static void cleanup() { + System.clearProperty(DIRECTORY_DELETE_ON_EXIT_PROPERTY); + FileSystemUtils.deleteRecursive(GEODE_WORKING_DIRECTORY); + stop(process); + } + + @Test + public void exportedJsonIsCorrect() throws Exception { + + File dataGolferJson = new File(GEODE_WORKING_DIRECTORY, DATA_GOLFERS_JSON); + + assertThat(dataGolferJson).isFile(); + + String actualJson = FileUtils.read(dataGolferJson); + + /* + String expectedJson = "[" + + "{\"@type\":\"example.app.golf.model.Golfer\",\"handicap\":9,\"id\":1,\"name\":\"John Blum\"}," + + "{\"@type\":\"example.app.golf.model.Golfer\",\"handicap\":10,\"id\":2,\"name\":\"Moe Haroon\"}" + + "]"; + + assertThat(actualJson).isEqualTo(expectedJson); + */ + + Set expectedGolfers = mapFromJsonToGolfers(actualJson); + + assertThat(expectedGolfers).isNotNull(); + assertThat(expectedGolfers).hasSize(2); + assertContains(expectedGolfers, Golfer.newGolfer(1L, "John Blum").withHandicap(9)); + assertContains(expectedGolfers, Golfer.newGolfer(2L, "Moe Haroon").withHandicap(10)); + } + + private void assertContains(Iterable golfers, Golfer golfer) { + + assertThat(StreamSupport.stream(golfers.spliterator(), false) + .anyMatch(it -> it.getId().equals(golfer.getId()) + && it.getName().equals(golfer.getName()) + && it.getHandicap().equals(golfer.getHandicap()))) + .isTrue(); + } + + private Set mapFromJsonToGolfers(String json) throws Exception { + return new HashSet<>(newObjectMapper().readerForListOf(Golfer.class).readValue(json)); + } + + private ObjectMapper newObjectMapper() { + + return new ObjectMapper() + .configure(DeserializationFeature.FAIL_ON_IGNORED_PROPERTIES, false) + .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); + } + + @Profile("EXPORT") + @SpringBootApplication + @EnableClusterAware + @EnableEntityDefinedRegions(basePackageClasses = Golfer.class) + @SuppressWarnings("unused") + static class TestGeodeConfiguration { + + public static void main(String[] args) { + + new SpringApplicationBuilder(TestGeodeConfiguration.class) + .web(WebApplicationType.NONE) + .build() + .run(args); + } + + private static void log(String message, Object... args) { + System.out.printf(String.format("%s%n", message), args); + System.out.flush(); + } + + @Bean + ApplicationRunner runner(GemfireTemplate golfersTemplate) { + + return args -> { + + save(golfersTemplate, Golfer.newGolfer(1L, "John Blum").withHandicap(9)); + save(golfersTemplate, Golfer.newGolfer(2L, "Moe Haroon").withHandicap(10)); + + log("FORE!"); + }; + } + + private Golfer save(GemfireTemplate golfersTemplate, Golfer golfer) { + golfersTemplate.put(golfer.getId(), golfer); + return golfer; + } + } +}