refactor: remove code for parsing site parsers configuration from file.

Addressed to #975

Author: php-coder

src/main/java/ru/mystamps/web/feature/series/importing/event/EventsConfig.java

@@ -17,37 +17,23 @@
  */
 package ru.mystamps.web.feature.series.importing.event;
 
-import java.util.HashMap;
-import java.util.Map;
-
-import javax.annotation.PostConstruct;
-
-import org.apache.commons.lang3.StringUtils;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import org.springframework.beans.factory.config.ConfigurableBeanFactory;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.context.ApplicationListener;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
-import org.springframework.core.env.ConfigurableEnvironment;
-import org.springframework.core.env.EnumerablePropertySource;
-import org.springframework.core.env.PropertySource;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
 
 import lombok.RequiredArgsConstructor;
 
 import ru.mystamps.web.config.ServicesConfig;
 import ru.mystamps.web.feature.series.importing.SeriesImportService;
 import ru.mystamps.web.feature.series.importing.extractor.JdbcSiteParserDao;
-import ru.mystamps.web.feature.series.importing.extractor.JsoupSiteParser;
-import ru.mystamps.web.feature.series.importing.extractor.SiteParser;
 import ru.mystamps.web.feature.series.importing.extractor.SiteParserDao;
 import ru.mystamps.web.feature.series.importing.extractor.SiteParserService;
 import ru.mystamps.web.feature.series.importing.extractor.SiteParserServiceImpl;
-import ru.mystamps.web.feature.series.importing.extractor.TimedSiteParser;
 
 @Configuration
 @RequiredArgsConstructor
@@ -58,25 +44,8 @@ public class EventsConfig {
 	private final SeriesImportService seriesImportService;
 	private final ServicesConfig servicesConfig;
 	private final ApplicationEventPublisher eventPublisher;
-	private final ConfigurableBeanFactory beanFactory;
-	private final ConfigurableEnvironment env;
 	private final NamedParameterJdbcTemplate jdbcTemplate;
 
-	@PostConstruct
-	public void init() {
-		Map<Integer, SiteParser> parsers = createSiteParsers();
-		for (Map.Entry<Integer, SiteParser> entry : parsers.entrySet()) {
-			Integer num = entry.getKey();
-			SiteParser parser = entry.getValue();
-			if (!parser.isFullyInitialized()) {
-				LOG.warn("Ignored non-fully initialized site parser (app.site-parser[{}])", num);
-				continue;
-			}
-			LOG.trace("Registering site parser for '{}'", parser);
-			beanFactory.registerSingleton("siteParser" + num, parser);
-		}
-	}
-	
 	@Bean
 	public SiteParserService siteParserService(SiteParserDao siteParserDao) {
 		return new SiteParserServiceImpl(
@@ -121,74 +90,4 @@ public ApplicationListener<ParsingFailed> getParsingFailedEventListener() {
 		);
 	}
 
-	@SuppressWarnings({
-		"PMD.AvoidInstantiatingObjectsInLoops",
-		"PMD.ModifiedCyclomaticComplexity" // TODO: deal with it someday
-	})
-	private Map<Integer, SiteParser> createSiteParsers() {
-		boolean foundSiteParserProps = false;
-		Map<Integer, SiteParser> parsers = new HashMap<>();
-		
-		for (PropertySource<?> source : env.getPropertySources()) {
-			// while we expect that properties will be in PropertiesPropertySource, we use
-			// EnumerablePropertySource to also handle systemProperties and systemEnvironment
-			// that are MapPropertySource and SystemEnvironmentPropertySource respectively
-			if (!(source instanceof EnumerablePropertySource<?>)) {
-				LOG.trace("Ignored property source: {} ({})", source.getName(), source.getClass());
-				continue;
-			}
-			
-			LOG.trace("Inspecting property source: {} ({})", source.getName(), source.getClass());
-			
-			for (String name : ((EnumerablePropertySource<?>)source).getPropertyNames()) {
-				if (!name.startsWith("app.site-parser")) {
-					continue;
-				}
-				
-				String propertyValue = (String)source.getProperty(name);
-				LOG.trace("Detected property '{}' with value {}", name, propertyValue);
-				
-				// extract parser number (app.site-parser[2].name -> 2)
-				String strNum = StringUtils.substringBetween(name, "[", "]");
-				if (StringUtils.isBlank(strNum)) {
-					LOG.warn("Ignored property '{}': could not extract index", name);
-					continue;
-				}
-				
-				Integer num = Integer.valueOf(strNum);
-				if (!parsers.containsKey(num)) {
-					// @todo #801 EventsConfig.createSiteParsers(): split the logic for properties
-					//  parsing and the object instantiation
-					SiteParser parser =
-						new TimedSiteParser(
-							LoggerFactory.getLogger(TimedSiteParser.class),
-							new JsoupSiteParser()
-						);
-					parsers.put(num, parser);
-				}
-				
-				// extract parser property (app.site-parser[2].name -> name)
-				String fieldName = StringUtils.substringAfterLast(name, ".");
-				if (StringUtils.isBlank(fieldName)) {
-					LOG.warn("Ignored property '{}': could not extract property name", name);
-					continue;
-				}
-				
-				SiteParser parser = parsers.get(num);
-				boolean validProperty = parser.setField(fieldName, propertyValue);
-				if (!validProperty) {
-					LOG.warn("Ignored property '{}': unknown or unsupported", name);
-					continue;
-				}
-				foundSiteParserProps = true;
-			}
-			// we shouldn't process others to be able to override a property
-			if (foundSiteParserProps) {
-				break;
-			}
-		}
-		
-		return parsers;
-	}
-	
 }

src/main/java/ru/mystamps/web/feature/series/importing/extractor/JsoupSiteParser.java

@@ -30,29 +30,23 @@
 
 import lombok.AccessLevel;
 import lombok.Getter;
-import lombok.NoArgsConstructor;
+import lombok.RequiredArgsConstructor;
 import lombok.Setter;
 
-// Getters/setters are being used in unit tests
+// Getters/setters/no-arg constructor are being used in unit tests
 @Getter(AccessLevel.PROTECTED)
 @Setter(AccessLevel.PROTECTED)
+@RequiredArgsConstructor(access = AccessLevel.PACKAGE)
 @SuppressWarnings({
 	"PMD.GodClass",
 	"PMD.TooManyMethods",
-	// false positive because setField() modifies the fields
+	// the fields are effectively final but I keep them non-final to simplify testing
 	"PMD.ImmutableField"
 })
-// TODO: consider to remove no-arg constructor after removing code for creating parsers from file
-@NoArgsConstructor
 public class JsoupSiteParser implements SiteParser {
 	private static final Logger LOG = LoggerFactory.getLogger(JsoupSiteParser.class);
 
-	// When you're adding a new field don't forget to also update:
-	// - JsoupSiteParser.setField()
-	// - JsoupSiteParser.isFullyInitialized() (optionally)
-	// - JsoupSiteParserTest.describe()
-	// - JsoupSiteParser constructor
-	// - SiteParserConfiguration
+	// When you add a new field, don't forget to also update SiteParserConfiguration.
 	private String name;
 	private String matchedUrl;
 	private String categoryLocator;
@@ -80,83 +74,6 @@ public JsoupSiteParser(SiteParserConfiguration cfg) {
 		currencyValue           = cfg.getCurrencyValue();
 	}
 
-	@Override
-	public boolean setField(String name, String value) {
-		Validate.validState(StringUtils.isNotBlank(name), "Field name must be non-blank");
-		Validate.validState(StringUtils.isNotBlank(value), "Field value must be non-blank");
-		
-		boolean valid = true;
-		
-		switch (name) {
-			
-			case "name":
-				setName(value);
-				break;
-			
-			case "matched-url":
-				setMatchedUrl(value);
-				break;
-			
-			case "category-locator":
-				setCategoryLocator(value);
-				break;
-			
-			case "country-locator":
-				setCountryLocator(value);
-				break;
-			
-			case "short-description-locator":
-				setShortDescriptionLocator(value);
-				break;
-			
-			case "image-url-locator":
-				setImageUrlLocator(value);
-				break;
-			
-			case "image-url-attribute":
-				setImageUrlAttribute(value);
-				break;
-			
-			case "issue-date-locator":
-				setIssueDateLocator(value);
-				break;
-			
-			case "seller-locator":
-				setSellerLocator(value);
-				break;
-			
-			case "price-locator":
-				setPriceLocator(value);
-				break;
-			
-			case "currency-value":
-				// @todo #695 Series import: validate app.site-parser[x].currency-value
-				setCurrencyValue(value);
-				break;
-			
-			default:
-				valid = false;
-				break;
-		}
-		
-		return valid;
-	}
-	
-	@Override
-	public boolean isFullyInitialized() {
-		return name != null
-			&& matchedUrl != null
-			&& (
-				categoryLocator != null
-				|| countryLocator != null
-				|| shortDescriptionLocator != null
-				|| imageUrlLocator != null
-				|| issueDateLocator != null
-				|| sellerLocator != null
-				|| priceLocator != null
-			);
-	}
-	
 	/**
 	 * Parse HTML document to get info about series.
 	 *

src/main/java/ru/mystamps/web/feature/series/importing/extractor/SiteParser.java

@@ -18,10 +18,5 @@
 package ru.mystamps.web.feature.series.importing.extractor;
 
 public interface SiteParser {
-	// TODO: remove because it's only needed for migrating configuration from file to database
-	boolean setField(String name, String value);
-	
-	boolean isFullyInitialized();
-	
 	SeriesInfo parse(String htmlPage);
 }

src/main/java/ru/mystamps/web/feature/series/importing/extractor/TimedSiteParser.java

@@ -31,16 +31,6 @@ public class TimedSiteParser implements SiteParser {
 	private final Logger log;
 	private final SiteParser parser;
 
-	@Override
-	public boolean setField(String name, String value) {
-		return parser.setField(name, value);
-	}
-	
-	@Override
-	public boolean isFullyInitialized() {
-		return parser.isFullyInitialized();
-	}
-	
 	@Override
 	public SeriesInfo parse(String htmlPage) {
 		// Why we don't use Spring's StopWatch?