From dd210c8e7014f919269bdf74cd353b474c027b76 Mon Sep 17 00:00:00 2001 From: Alex Osborne Date: Wed, 20 Nov 2024 16:30:15 +0900 Subject: [PATCH] Replace Spring's removed @Required annotation with our own Spring 6 removed @Required and they suggest using constructor injection instead. If we switched our beans to that we'd break existing Heritrix crawl configs. So this change implements our own basic version so we still get errors when a @Required property is null. --- .../org/archive/checkpointing/Checkpoint.java | 2 +- .../java/org/archive/spring/ConfigPath.java | 1 - .../archive/spring/PathSharingContext.java | 12 ++++++ .../java/org/archive/spring/Required.java | 16 ++++++++ .../RequiredAnnotationBeanPostProcessor.java | 40 +++++++++++++++++++ .../main/java/org/archive/spring/Sheet.java | 2 - .../spring/DecideRuledSheetAssociation.java | 2 +- .../spring/SurtPrefixesSheetAssociation.java | 4 +- .../archive/modules/ScriptedProcessor.java | 2 +- .../deciderules/ScriptedDecideRule.java | 2 +- .../ViaSurtPrefixedDecideRule.java | 2 +- .../archive/modules/seeds/TextSeedModule.java | 2 +- 12 files changed, 76 insertions(+), 11 deletions(-) create mode 100644 commons/src/main/java/org/archive/spring/Required.java create mode 100644 commons/src/main/java/org/archive/spring/RequiredAnnotationBeanPostProcessor.java diff --git a/commons/src/main/java/org/archive/checkpointing/Checkpoint.java b/commons/src/main/java/org/archive/checkpointing/Checkpoint.java index 4117a993..a62bed41 100644 --- a/commons/src/main/java/org/archive/checkpointing/Checkpoint.java +++ b/commons/src/main/java/org/archive/checkpointing/Checkpoint.java @@ -30,11 +30,11 @@ import java.util.logging.Logger; import org.apache.commons.io.FileUtils; import org.archive.spring.ConfigPath; +import org.archive.spring.Required; import org.archive.util.ArchiveUtils; import org.json.JSONException; import org.json.JSONObject; import org.springframework.beans.factory.InitializingBean; -import org.springframework.beans.factory.annotation.Required; /** * Represents a single checkpoint, by its name and main store directory. diff --git a/commons/src/main/java/org/archive/spring/ConfigPath.java b/commons/src/main/java/org/archive/spring/ConfigPath.java index 276a7ac4..fdb47573 100644 --- a/commons/src/main/java/org/archive/spring/ConfigPath.java +++ b/commons/src/main/java/org/archive/spring/ConfigPath.java @@ -41,7 +41,6 @@ import java.io.Serializable; * See the License for the specific language governing permissions and * limitations under the License. */ -import org.springframework.beans.factory.annotation.Required; /** * A filesystem path, as a bean, for the convenience of configuration diff --git a/commons/src/main/java/org/archive/spring/PathSharingContext.java b/commons/src/main/java/org/archive/spring/PathSharingContext.java index e4957042..edd1898f 100644 --- a/commons/src/main/java/org/archive/spring/PathSharingContext.java +++ b/commons/src/main/java/org/archive/spring/PathSharingContext.java @@ -68,18 +68,28 @@ public class PathSharingContext extends FileSystemXmlApplicationContext { public PathSharingContext(String configLocation) throws BeansException { super(configLocation); + init(); } public PathSharingContext(String[] configLocations, ApplicationContext parent) throws BeansException { super(configLocations, parent); + init(); } public PathSharingContext(String[] configLocations, boolean refresh, ApplicationContext parent) throws BeansException { super(configLocations, refresh, parent); + init(); } public PathSharingContext(String[] configLocations, boolean refresh) throws BeansException { super(configLocations, refresh); + init(); } public PathSharingContext(String[] configLocations) throws BeansException { super(configLocations); + init(); + } + + private void init() { + // enforce @Required annotation + addBeanFactoryPostProcessor(beanFactory -> beanFactory.addBeanPostProcessor(new RequiredAnnotationBeanPostProcessor())); } public String getPrimaryConfigurationPath() { @@ -201,4 +211,6 @@ public class PathSharingContext extends FileSystemXmlApplicationContext { } return data; } + + } diff --git a/commons/src/main/java/org/archive/spring/Required.java b/commons/src/main/java/org/archive/spring/Required.java new file mode 100644 index 00000000..7b5dc053 --- /dev/null +++ b/commons/src/main/java/org/archive/spring/Required.java @@ -0,0 +1,16 @@ +package org.archive.spring; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Replacement for the removed Spring @Required annotation. Do not use this in new beans, + * use constructor injection instead. This only exists to avoid breaking existing crawl profiles. + */ +@Deprecated +@Target(ElementType.METHOD) +@Retention(RetentionPolicy.RUNTIME) +public @interface Required { +} diff --git a/commons/src/main/java/org/archive/spring/RequiredAnnotationBeanPostProcessor.java b/commons/src/main/java/org/archive/spring/RequiredAnnotationBeanPostProcessor.java new file mode 100644 index 00000000..f422a60d --- /dev/null +++ b/commons/src/main/java/org/archive/spring/RequiredAnnotationBeanPostProcessor.java @@ -0,0 +1,40 @@ +package org.archive.spring; + +import org.springframework.beans.factory.BeanInitializationException; +import org.springframework.beans.factory.config.BeanPostProcessor; + +import java.lang.reflect.Method; + +public class RequiredAnnotationBeanPostProcessor implements BeanPostProcessor { + @Override + public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeanInitializationException { + for (Method method : bean.getClass().getMethods()) { + if (method.isAnnotationPresent(Required.class)) { + Method getter; + + if (method.getName().startsWith("set")) { + String getterName = "get" + method.getName().substring(3); + try { + getter = bean.getClass().getMethod(getterName); + } catch (NoSuchMethodException e) { + throw new BeanInitializationException("Missing getter for @Required property on bean: " + + beanName + " for method: " + method.getName()); + } + } else { + getter = method; + } + + try { + Object value = getter.invoke(bean); + if (value == null) { + throw new BeanInitializationException("Property not set for bean: " + beanName + + " on method: " + method.getName()); + } + } catch (Exception e) { + throw new BeanInitializationException("Error processing @Required for bean: " + beanName, e) {}; + } + } + } + return bean; + } +} diff --git a/commons/src/main/java/org/archive/spring/Sheet.java b/commons/src/main/java/org/archive/spring/Sheet.java index 630275fb..79f57542 100644 --- a/commons/src/main/java/org/archive/spring/Sheet.java +++ b/commons/src/main/java/org/archive/spring/Sheet.java @@ -28,8 +28,6 @@ import org.springframework.beans.TypeMismatchException; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.BeanNameAware; -import org.springframework.beans.factory.annotation.Required; - /** * Collection of overrides: alternative values for object properties diff --git a/engine/src/main/java/org/archive/crawler/spring/DecideRuledSheetAssociation.java b/engine/src/main/java/org/archive/crawler/spring/DecideRuledSheetAssociation.java index 1e6120dc..bf01ca9c 100644 --- a/engine/src/main/java/org/archive/crawler/spring/DecideRuledSheetAssociation.java +++ b/engine/src/main/java/org/archive/crawler/spring/DecideRuledSheetAssociation.java @@ -20,8 +20,8 @@ package org.archive.crawler.spring; import org.archive.modules.deciderules.DecideRule; +import org.archive.spring.Required; import org.springframework.beans.factory.BeanNameAware; -import org.springframework.beans.factory.annotation.Required; import org.springframework.core.Ordered; /** diff --git a/engine/src/main/java/org/archive/crawler/spring/SurtPrefixesSheetAssociation.java b/engine/src/main/java/org/archive/crawler/spring/SurtPrefixesSheetAssociation.java index 2e0da7e3..a77e6a63 100644 --- a/engine/src/main/java/org/archive/crawler/spring/SurtPrefixesSheetAssociation.java +++ b/engine/src/main/java/org/archive/crawler/spring/SurtPrefixesSheetAssociation.java @@ -19,9 +19,9 @@ package org.archive.crawler.spring; -import java.util.List; +import org.archive.spring.Required; -import org.springframework.beans.factory.annotation.Required; +import java.util.List; /** * SheetAssociation applied on the basis of matching SURT prefixes. diff --git a/modules/src/main/java/org/archive/modules/ScriptedProcessor.java b/modules/src/main/java/org/archive/modules/ScriptedProcessor.java index f8c7215c..af74f580 100644 --- a/modules/src/main/java/org/archive/modules/ScriptedProcessor.java +++ b/modules/src/main/java/org/archive/modules/ScriptedProcessor.java @@ -29,10 +29,10 @@ import javax.script.ScriptException; import org.apache.commons.io.IOUtils; import org.archive.io.ReadSource; +import org.archive.spring.Required; import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanInitializationException; import org.springframework.beans.factory.InitializingBean; -import org.springframework.beans.factory.annotation.Required; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; diff --git a/modules/src/main/java/org/archive/modules/deciderules/ScriptedDecideRule.java b/modules/src/main/java/org/archive/modules/deciderules/ScriptedDecideRule.java index 3092bbe6..6a70237a 100644 --- a/modules/src/main/java/org/archive/modules/deciderules/ScriptedDecideRule.java +++ b/modules/src/main/java/org/archive/modules/deciderules/ScriptedDecideRule.java @@ -30,10 +30,10 @@ import javax.script.ScriptException; import org.apache.commons.io.IOUtils; import org.archive.io.ReadSource; import org.archive.modules.CrawlURI; +import org.archive.spring.Required; import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanInitializationException; import org.springframework.beans.factory.InitializingBean; -import org.springframework.beans.factory.annotation.Required; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; diff --git a/modules/src/main/java/org/archive/modules/deciderules/ViaSurtPrefixedDecideRule.java b/modules/src/main/java/org/archive/modules/deciderules/ViaSurtPrefixedDecideRule.java index c3fb8c98..b2fa325c 100644 --- a/modules/src/main/java/org/archive/modules/deciderules/ViaSurtPrefixedDecideRule.java +++ b/modules/src/main/java/org/archive/modules/deciderules/ViaSurtPrefixedDecideRule.java @@ -23,8 +23,8 @@ import java.util.ArrayList; import java.util.List; import org.archive.modules.CrawlURI; +import org.archive.spring.Required; import org.archive.util.SurtPrefixSet; -import org.springframework.beans.factory.annotation.Required; /** * Rule applies the configured decision for any URI which has a 'via' whose diff --git a/modules/src/main/java/org/archive/modules/seeds/TextSeedModule.java b/modules/src/main/java/org/archive/modules/seeds/TextSeedModule.java index 845d0699..be59ecf8 100644 --- a/modules/src/main/java/org/archive/modules/seeds/TextSeedModule.java +++ b/modules/src/main/java/org/archive/modules/seeds/TextSeedModule.java @@ -36,12 +36,12 @@ import org.archive.modules.CrawlURI; import org.archive.modules.SchedulingConstants; import org.archive.net.UURI; import org.archive.net.UURIFactory; +import org.archive.spring.Required; import org.archive.spring.WriteTarget; import org.archive.util.ArchiveUtils; import org.archive.util.DevUtils; import org.archive.util.iterator.LineReadingIterator; import org.archive.util.iterator.RegexLineIterator; -import org.springframework.beans.factory.annotation.Required; /** * Module that announces a list of seeds from a text source (such