Implemented simple resilience approach and handle now 400 retry merce…

…des-benz#100

- Introducing a simple resilience approach (which can handle
  retry and fall through mechanism, but we currently have only one
  retry mechanism)
- having a 400 BAD request on a checkmarx scan upload can be resulted
  by an internal server error on checkmarx side. Normally this only
  happens in test scenarios, but wanted to have this stable. So an
  automated retry is triggered 3 times before throwing an exception now

sechub-adapter/src/main/java/com/daimler/sechub/adapter/AbstractAdapterConfig.java

@@ -17,6 +17,7 @@ public abstract class AbstractAdapterConfig implements AdapterConfig {
 	private SealedObject passwordOrAPITokenBase64encoded;
 
 	int timeToWaitForNextCheckOperationInMilliseconds;
+	/* TODO Albert Tregnaghi, 2019-12-04: at the moment the time out settings seems to be not used. Also its corelated with check results. There is something odd should be checked*/
 	int timeOutInMilliseconds;
 	int proxyPort;
 	String proxyHostname;

sechub-adapter/src/main/java/com/daimler/sechub/adapter/AbstractSpringRestAdapterContext.java

@@ -22,7 +22,7 @@
 
 /**
  * Context for REST execution per spring REST templates (per default with a simple String as result).
- * 
+ *
  * @author Albert Tregnaghi
  *
  */
@@ -36,9 +36,9 @@ public AbstractSpringRestAdapterContext(C config, A adapter) {
 		/* setup dedicated rest template */
 
 		ClientHttpRequestFactory requestFactory = createRequestFactory(config);
-		
+
 		restTemplate = new RestTemplate(requestFactory);
-		
+
 		restTemplate.getMessageConverters().addAll(createMessageConverters());
 
 		List<ClientHttpRequestInterceptor> interceptors = new ArrayList<>();
@@ -47,7 +47,7 @@ public AbstractSpringRestAdapterContext(C config, A adapter) {
 		if (interceptor != null) {
 			interceptors.add(interceptor);
 		}
-		
+
 		Object obj = config.getOptions().get(SECHUB_OPTION_CLIENTHTTPREQUESTINTERCEPTOR);
 		if (obj instanceof ClientHttpRequestInterceptor) {
 			ClientHttpRequestInterceptor optionInterceptor = (ClientHttpRequestInterceptor) obj;
@@ -56,14 +56,14 @@ public AbstractSpringRestAdapterContext(C config, A adapter) {
 		restTemplate.setInterceptors(interceptors);
 
 	}
-	
+
 	private ClientHttpRequestFactory createRequestFactory(C config) {
 		ClientHttpRequestFactory factory = null;
 		if (! config.isTrustAllCertificatesEnabled()) {
 			factory = createStandardSpringRequestFactory(config);
 		}else {
 			factory = new TrustAllSupport(getAdapter(),config).createTrustAllFactory();
-			
+
 		}
 		/* we create buffering variant, so we can do trace logging if necessary - see TraceLogClientHTTPRequestInterceptor*/
 		return new BufferingClientHttpRequestFactory(factory);
@@ -80,7 +80,7 @@ private SimpleClientHttpRequestFactory createStandardSpringRequestFactory(C conf
 		return requestFactory;
 	}
 
-	
+
 
 	private Set<HttpMessageConverter<?>> createMessageConverters() {
 		Set<HttpMessageConverter<?>> set = new HashSet<>();

sechub-doc/src/docs/asciidoc/diagrams/diagram_simple_resilience.plantuml

@@ -0,0 +1,32 @@
+interface ResilienceConsultant{
+    {method} proposalFor (ResilienceContext): ResilienceProposal
+}
+interface ResilienceContext
+
+interface ResilienceProposal
+
+interface RetryResilienceProposal extends ResilienceProposal{
+ {method} int getMaximumAmountOfRetries
+ {method} int getMillisecondsToWaitBeforeRetry
+}
+
+interface FallThroughResilienceProposal extends ResilienceProposal{
+ {method} int getMillisecondsForFallThrough
+}
+
+interface ActionWhichShallBeResilient{
+    <R> execute() throws Exception
+}
+
+class ResilientActionExecutor
+
+ResilienceConsultant .. ResilienceContext
+ResilientActionExecutor --> "creates" ResilienceContext
+
+ResilienceConsultant ..> "returns" ResilienceProposal
+
+
+ResilientActionExecutor "1" -- "0..*" ResilienceConsultant
+ResilientActionExecutor --> "inspects" ResilienceProposal
+
+ResilientActionExecutor  --> "executes" ActionWhichShallBeResilient

sechub-doc/src/docs/asciidoc/documents/architecture/08_concepts.adoc

@@ -2,17 +2,13 @@
 [[section-concepts]]
 == Cross-cutting Concepts
 
-=== Simple Domain Driven Design (SDDD)
+=== Domain Driven Design (SDDD)
 
 include::../shared/concepts/concept_simple_domain_driven_design.adoc[]
 
 
-=== _<Concept 2>_
+=== Resilience
 
-_<explanation>_
+include::../shared/concepts/concept_simple_resilience.adoc[]
 
-...
 
-=== _<Concept n>_
-
-_<explanation>_

sechub-doc/src/docs/asciidoc/documents/shared/concepts/concept_simple_resilience.adoc

@@ -0,0 +1,18 @@
+We do currently not use Hystrix or another framework to handle resilience,
+but use a simple own approach:
+
+plantuml::./diagrams/diagram_simple_resilience.plantuml[]
+
+A resilient action executor can be used to execute a dedicated
+action. Implementation of `ResilienctConsultant` will give
+dedicated proposals how to handle an error.
+
+There are currently two different ways:
+
+- Retry +
+  In this case the failed action will be retried some times
+  before the exception is thrown
+- Fall through +
+  Here the failure is recognized as a situation which will be
+  happen very often because a fix will take some time and so
+  to fail fast for a given time period instead of blocking

sechub-scan-product-checkmarx/src/main/java/com/daimler/sechub/domain/scan/product/checkmarx/CheckmarxBadRequestConsultant.java

@@ -0,0 +1,34 @@
+package com.daimler.sechub.domain.scan.product.checkmarx;
+
+import java.util.Objects;
+
+import org.springframework.web.client.HttpClientErrorException;
+
+import com.daimler.sechub.sharedkernel.resilience.ResilienceConsultant;
+import com.daimler.sechub.sharedkernel.resilience.ResilienceContext;
+import com.daimler.sechub.sharedkernel.resilience.ResilienceProposal;
+import com.daimler.sechub.sharedkernel.resilience.SimpleRetryResilienceProposal;
+import com.daimler.sechub.sharedkernel.util.StacktraceUtil;
+
+public class CheckmarxBadRequestConsultant implements ResilienceConsultant {
+
+	@Override
+	public ResilienceProposal consultFor(ResilienceContext context) {
+		Objects.requireNonNull(context);
+		Throwable rootCause = StacktraceUtil.findRootCause(context.getCurrentError());
+		if (rootCause instanceof HttpClientErrorException) {
+			HttpClientErrorException hce = (HttpClientErrorException) rootCause;
+			int statusCode = hce.getRawStatusCode();
+			if (statusCode == 400) {
+				/*
+				 * BAD request - this can happen for same project scans put to queue because
+				 * there can a chmkx server error happen
+				 */
+				return new SimpleRetryResilienceProposal("checkmarx bad request handling", 3, 2000);
+
+			}
+		}
+		return null;
+	}
+
+}

sechub-scan-product-checkmarx/src/main/java/com/daimler/sechub/domain/scan/product/checkmarx/CheckmarxProductExecutor.java

@@ -23,6 +23,7 @@
 import com.daimler.sechub.domain.scan.product.ProductResult;
 import com.daimler.sechub.sharedkernel.MustBeDocumented;
 import com.daimler.sechub.sharedkernel.execution.SecHubExecutionContext;
+import com.daimler.sechub.sharedkernel.resilience.ResilientActionExecutor;
 import com.daimler.sechub.sharedkernel.storage.StorageService;
 import com.daimler.sechub.storage.core.JobStorage;
 
@@ -48,42 +49,52 @@ public class CheckmarxProductExecutor extends AbstractCodeScanProductExecutor<Ch
 	@Autowired
 	StorageService storageService;
 
+	ResilientActionExecutor<ProductResult> resilientActionExecutor;
+
+	public CheckmarxProductExecutor() {
+		/* we create here our own instance - only for this service!*/
+		this.resilientActionExecutor=new ResilientActionExecutor<>();
+		this.resilientActionExecutor.add(new CheckmarxBadRequestConsultant());
+
+	}
+
 	@Override
-	protected List<ProductResult> executeWithAdapter(SecHubExecutionContext context, CheckmarxInstallSetup setup, TargetRegistryInfo data)
-			throws Exception {
+	protected List<ProductResult> executeWithAdapter(SecHubExecutionContext context, CheckmarxInstallSetup setup, TargetRegistryInfo data) throws Exception {
 		LOG.debug("Trigger checkmarx adapter execution");
 
 		UUID jobUUID = context.getSechubJobUUID();
 		String projectId = context.getConfiguration().getProjectId();
 
 		JobStorage storage = storageService.getJobStorage(projectId, jobUUID);
-		try(InputStream sourceCodeZipFileInputStream = storage.fetch("sourcecode.zip")){
-
-			/* @formatter:off */
-
-			CheckmarxAdapterConfig checkMarxConfig =CheckmarxConfig.builder().
-					configure(new OneInstallSetupConfigBuilderStrategy(setup)).
-					setTimeToWaitForNextCheckOperationInMinutes(scanResultCheckPeriodInMinutes).
-					setScanResultTimeOutInMinutes(scanResultCheckTimeOutInMinutes).
-					setFileSystemSourceFolders(data.getCodeUploadFileSystemFolders()).
-					setSourceCodeZipFileInputStream(sourceCodeZipFileInputStream).
-					setTeamIdForNewProjects(setup.getTeamIdForNewProjects()).
-					setProjectId(projectId).
-					setTraceID(context.getTraceLogIdAsString()).
-					/* TODO Albert Tregnaghi, 2018-10-09:policy id - always default id - what about config.getPoliciyID() ?!?! */
-					build();
-			/* @formatter:on */
-
-			/* execute checkmarx by adapter and return product result */
-			String xml = checkmarxAdapter.start(checkMarxConfig);
-			ProductResult result = new ProductResult(context.getSechubJobUUID(),projectId, getIdentifier(), xml);
-			return Collections.singletonList(result);
-		}
+		ProductResult result = resilientActionExecutor.executeResilient(()-> {
+
+				try (InputStream sourceCodeZipFileInputStream = storage.fetch("sourcecode.zip")) {
+
+					/* @formatter:off */
+
+					CheckmarxAdapterConfig checkMarxConfig =CheckmarxConfig.builder().
+							configure(new OneInstallSetupConfigBuilderStrategy(setup)).
+							setTimeToWaitForNextCheckOperationInMinutes(scanResultCheckPeriodInMinutes).
+							setScanResultTimeOutInMinutes(scanResultCheckTimeOutInMinutes).
+							setFileSystemSourceFolders(data.getCodeUploadFileSystemFolders()).
+							setSourceCodeZipFileInputStream(sourceCodeZipFileInputStream).
+							setTeamIdForNewProjects(setup.getTeamIdForNewProjects()).
+							setProjectId(projectId).
+							setTraceID(context.getTraceLogIdAsString()).
+							/* TODO Albert Tregnaghi, 2018-10-09:policy id - always default id - what about config.getPoliciyID() ?!?! */
+							build();
+					/* @formatter:on */
+
+					/* execute checkmarx by adapter and return product result */
+					String xml = checkmarxAdapter.start(checkMarxConfig);
+					ProductResult productResult = new ProductResult(context.getSechubJobUUID(), projectId, getIdentifier(), xml);
+					return productResult;
+				}
+		});
+		return Collections.singletonList(result);
 
 	}
 
-
-
 	@Override
 	public ProductIdentifier getIdentifier() {
 		return ProductIdentifier.CHECKMARX;

sechub-scan-product-checkmarx/src/test/java/com/daimler/sechub/domain/scan/product/checkmarx/CheckmarxBadRequestConsultantTest.java

@@ -0,0 +1,65 @@
+package com.daimler.sechub.domain.scan.product.checkmarx;
+
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.http.HttpStatus;
+import org.springframework.web.client.HttpClientErrorException;
+
+import com.daimler.sechub.sharedkernel.resilience.ResilienceContext;
+import com.daimler.sechub.sharedkernel.resilience.ResilienceProposal;
+import com.daimler.sechub.sharedkernel.resilience.RetryResilienceProposal;
+
+public class CheckmarxBadRequestConsultantTest {
+
+	private CheckmarxBadRequestConsultant consultantToTest;
+	private ResilienceContext context;
+
+	@Before
+	public void before() {
+		consultantToTest = new CheckmarxBadRequestConsultant();
+		context= mock(ResilienceContext.class);
+	}
+
+	@Test
+	public void no_exception_returns_null() {
+		/* prepare*/
+
+		/* execute*/
+		ResilienceProposal proposal = consultantToTest.consultFor(context);
+
+		/* test*/
+		assertNull(proposal);
+	}
+
+	@Test
+	public void illegal_argument_exception_returns_null() {
+		/* prepare*/
+		when(context.getCurrentError()).thenReturn(new IllegalArgumentException());
+
+		/* execute*/
+		ResilienceProposal proposal = consultantToTest.consultFor(context);
+
+		/* test*/
+		assertNull(proposal);
+	}
+
+	@Test
+	public void http_bad_request_400_exception_returns_retry_proposal_with_3_retries_and_2000_millis_to_wait() {
+		/* prepare*/
+		when(context.getCurrentError()).thenReturn(new HttpClientErrorException(HttpStatus.BAD_REQUEST));
+
+		/* execute*/
+		ResilienceProposal proposal = consultantToTest.consultFor(context);
+
+		/* test*/
+		assertNotNull(proposal);
+		assertTrue(proposal instanceof RetryResilienceProposal);
+		RetryResilienceProposal rrp = (RetryResilienceProposal) proposal;
+		assertEquals(3, rrp.getMaximumAmountOfRetries());
+		assertEquals(2000, rrp.getMillisecondsToWaitBeforeRetry());
+	}
+
+}

sechub-scan-product-checkmarx/src/test/java/com/daimler/sechub/domain/scan/product/checkmarx/CheckmarxProductExecutorTest.java

@@ -0,0 +1,27 @@
+package com.daimler.sechub.domain.scan.product.checkmarx;
+
+import static org.junit.Assert.*;
+
+import org.junit.Before;
+import org.junit.Test;
+
+public class CheckmarxProductExecutorTest {
+
+	private CheckmarxProductExecutor executorToTest;
+
+	@Before
+	public void before() {
+		executorToTest= new CheckmarxProductExecutor();
+	}
+
+	@Test
+	public void action_executor_is_not_null() {
+		assertNotNull(executorToTest.resilientActionExecutor);
+	}
+
+	@Test
+	public void action_executor_contains_checkmarx_bad_request_consultant() {
+		assertTrue(executorToTest.resilientActionExecutor.containsConsultant(CheckmarxBadRequestConsultant.class));
+	}
+
+}

sechub-scan/src/main/java/com/daimler/sechub/domain/scan/product/AbstractProductExecutionService.java

@@ -134,7 +134,7 @@ protected void executeAndPersistResults(List<? extends ProductExecutor> executor
 	}
 
 	/**
-	 * Persists the result. This wil ALWAYS start a new transaction. So former
+	 * Persists the result. This will ALWAYS start a new transaction. So former
 	 * results will NOT get lost if this persistence fails. Necessary for debugging
 	 * and also the later possibility to relaunch already existing sechub jobs!
 	 * Reason: When a former scan did take a very long time and was done. The next

sechub-shared-kernel/src/main/java/com/daimler/sechub/sharedkernel/resilience/ActionWhichShallBeResilient.java

@@ -0,0 +1,11 @@
+package com.daimler.sechub.sharedkernel.resilience;
+
+public interface ActionWhichShallBeResilient<R> {
+
+	/**
+	 * Action method which shall be executed in resilient way
+	 * @return
+	 * @throws Exception
+	 */
+	public R execute() throws Exception;
+}

sechub-shared-kernel/src/main/java/com/daimler/sechub/sharedkernel/resilience/FallthroughResilienceProposal.java

@@ -0,0 +1,16 @@
+package com.daimler.sechub.sharedkernel.resilience;
+
+public interface FallthroughResilienceProposal extends ResilienceProposal{
+	/**
+	 *
+	 *Defines the amount of time (in milliseconds) where a fall through shall
+	 *be done. This means, the current error will be simply reused and given back.
+	 *<br><br>This is interesting when having a server done, were a time out (e.g.
+	 *HTTP request) appears only a some time (e.g. 2 minutes). And we got n amount of
+	 *calls (e.g. 500...) and we do NOT want to have every call wait for 2 Minutes but
+	 *instead fail fast. So interesting in this case could be a time of 1000*60*2 to do
+	 *the fall through.
+	 * @return milliseconds where a fall through shall be done
+	 */
+	public long getMillisecondsForFallThrough();
+}