constructor based injection, randomized values for testing, added constants

8 files changed, 59 insertions, 125 deletions

diff --git a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/Constants.java b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/Constants.java
index ce48a88e..9104c55f 100644
--- a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/Constants.java
+++ b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/Constants.java
@@ -171,4 +171,6 @@ public class Constants {
       "AJZyj/+sdCMDRq9RkvbFcgSTVn/OfS8EUE81ddwP8MNuJ1kd1SWBUJPaQX2JLJHrL54mkOhrkhH2M/zcuOTu8nW9TOEg"
       + "XGjrRB/0HpiYKpV+VDJViyyc/GacNLxN4Anw4pima6gHYaJIw9hQkL/nuO2hyh8PGJd7rxeFXJmbLy+X";
 
+  public static final String COUNTRY_CODE_DE = "DE";
+  public static final String COUNTRY_CODE_IT = "IT";
 }
diff --git a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/dao/SimpleEidasData.java b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/dao/SimpleEidasData.java
index 43d85772..674f5b48 100644
--- a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/dao/SimpleEidasData.java
+++ b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/dao/SimpleEidasData.java
@@ -83,7 +83,6 @@ public class SimpleEidasData {
     if (!result.getTaxNumber().equals(taxNumber)) {
       return false;
     }
-
     return true;
   }
 }
diff --git a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/ernb/DummyErnbClient.java b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/ernb/DummyErnbClient.java
index 978be4d0..2d2fa76d 100644
--- a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/ernb/DummyErnbClient.java
+++ b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/ernb/DummyErnbClient.java
@@ -11,16 +11,7 @@ public class DummyErnbClient implements IErnbClient {
 
   @Override
   public ArrayList<RegisterResult> searchWithPersonIdentifer(String personIdentifer) {
-    // NOTE: Are we using Mockito or these fixed strings for testing, why are those defined here?
-    switch (personIdentifer) {
-      case "a12345":
-      case "a12345-":
-        return result1();
-      case "a123456":
-        return result2();
-      default:
-        return resultEmpty();
-    }
+    return resultEmpty();
   }
 
   @Override
@@ -48,20 +39,5 @@ public class DummyErnbClient implements IErnbClient {
     return new ArrayList<RegisterResult>();//Nobody found
   }
 
-  private ArrayList<RegisterResult> result1() {
-    ArrayList<RegisterResult> results = new ArrayList<>();
-    RegisterResult result1 = new RegisterResult("a12345", "Tom", "Mustermann", "1950-01-01", "Wien");
-    results.add(result1);
-    RegisterResult result2 = new RegisterResult("a12345-", "Tom", "Mustermann", "1950-01-01", "Wien");
-    results.add(result2);
-    return results;
-  }
-
-  private ArrayList<RegisterResult> result2() {
-    ArrayList<RegisterResult> results = new ArrayList<>();
-    RegisterResult result = new RegisterResult("a123456", "Max", "Mustermann", "2000-01-01", "Wien");
-    results.add(result);
-    return results;
-  }
 
 }
diff --git a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/DeSpecificDetailSearchProcessor.java b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/DeSpecificDetailSearchProcessor.java
index b5e8551b..e8cb7a1a 100644
--- a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/DeSpecificDetailSearchProcessor.java
+++ b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/DeSpecificDetailSearchProcessor.java
@@ -1,17 +1,23 @@
 package at.asitplus.eidas.specific.modules.auth.eidas.v2.handler;
 
+import at.asitplus.eidas.specific.modules.auth.eidas.v2.Constants;
 import at.asitplus.eidas.specific.modules.auth.eidas.v2.dao.MergedRegisterSearchResult;
 import at.asitplus.eidas.specific.modules.auth.eidas.v2.dao.RegisterResult;
 import at.asitplus.eidas.specific.modules.auth.eidas.v2.dao.SimpleEidasData;
+import at.asitplus.eidas.specific.modules.auth.eidas.v2.ernb.IErnbClient;
+import at.asitplus.eidas.specific.modules.auth.eidas.v2.zmr.IZmrClient;
 
 import java.util.ArrayList;
 
 public class DeSpecificDetailSearchProcessor extends ICountrySpecificDetailSearchProcessor {
 
+  public DeSpecificDetailSearchProcessor(IErnbClient ernbClient, IZmrClient zmrClient) {
+    super(ernbClient, zmrClient);
+  }
+
   @Override
   public boolean canHandle(String countryCode, SimpleEidasData eidData) {
-    // NOTE: Please extract constant for "de"
-    if (!countryCode.equalsIgnoreCase("de")) {
+    if (!countryCode.equalsIgnoreCase(Constants.COUNTRY_CODE_DE)) {
       return false;
     }
     if (eidData.getBirthName() == null || eidData.getBirthName().isEmpty()) {
diff --git a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/ICountrySpecificDetailSearchProcessor.java b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/ICountrySpecificDetailSearchProcessor.java
index b9ab2ceb..6a2b2c0a 100644
--- a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/ICountrySpecificDetailSearchProcessor.java
+++ b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/ICountrySpecificDetailSearchProcessor.java
@@ -27,22 +27,14 @@ import at.asitplus.eidas.specific.modules.auth.eidas.v2.dao.MergedRegisterSearch
 import at.asitplus.eidas.specific.modules.auth.eidas.v2.dao.SimpleEidasData;
 import at.asitplus.eidas.specific.modules.auth.eidas.v2.ernb.IErnbClient;
 import at.asitplus.eidas.specific.modules.auth.eidas.v2.zmr.IZmrClient;
-import org.springframework.beans.factory.annotation.Autowired;
 
 public abstract class ICountrySpecificDetailSearchProcessor {
 
-  // NOTE: Please use constructor injection
-
   protected IErnbClient ernbClient;
   protected IZmrClient zmrClient;
 
-  @Autowired
-  public void setErnbClient(IErnbClient ernbClient) {
+  public ICountrySpecificDetailSearchProcessor(IErnbClient ernbClient, IZmrClient zmrClient) {
     this.ernbClient = ernbClient;
-  }
-
-  @Autowired
-  public void setZmrClient(IZmrClient zmrClient) {
     this.zmrClient = zmrClient;
   }
 
diff --git a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/ItSpecificDetailSearchProcessor.java b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/ItSpecificDetailSearchProcessor.java
index d055345a..a94a67b3 100644
--- a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/ItSpecificDetailSearchProcessor.java
+++ b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/ItSpecificDetailSearchProcessor.java
@@ -1,16 +1,23 @@
 package at.asitplus.eidas.specific.modules.auth.eidas.v2.handler;
 
+import at.asitplus.eidas.specific.modules.auth.eidas.v2.Constants;
 import at.asitplus.eidas.specific.modules.auth.eidas.v2.dao.MergedRegisterSearchResult;
 import at.asitplus.eidas.specific.modules.auth.eidas.v2.dao.RegisterResult;
 import at.asitplus.eidas.specific.modules.auth.eidas.v2.dao.SimpleEidasData;
+import at.asitplus.eidas.specific.modules.auth.eidas.v2.ernb.IErnbClient;
+import at.asitplus.eidas.specific.modules.auth.eidas.v2.zmr.IZmrClient;
 
 import java.util.ArrayList;
 
 public class ItSpecificDetailSearchProcessor extends ICountrySpecificDetailSearchProcessor {
 
+  public ItSpecificDetailSearchProcessor(IErnbClient ernbClient, IZmrClient zmrClient) {
+    super(ernbClient, zmrClient);
+  }
+
   @Override
   public boolean canHandle(String countryCode, SimpleEidasData eidData) {
-    if (!countryCode.equalsIgnoreCase("it")) {
+    if (!countryCode.equalsIgnoreCase(Constants.COUNTRY_CODE_IT)) {
       return false;
     }
     if (eidData.getTaxNumber() == null || eidData.getTaxNumber().isEmpty()) {
diff --git a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/tasks/InitialSearchTask.java b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/tasks/InitialSearchTask.java
index 20ee9325..a87be6c5 100644
--- a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/tasks/InitialSearchTask.java
+++ b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/tasks/InitialSearchTask.java
@@ -49,7 +49,6 @@ import eu.eidas.auth.commons.protocol.eidas.impl.PostalAddress;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
 import org.joda.time.DateTime;
-import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
 import javax.servlet.http.HttpServletRequest;
@@ -72,9 +71,6 @@ public class InitialSearchTask extends AbstractAuthServletTask {
 
   private List<ICountrySpecificDetailSearchProcessor> handlers = new ArrayList<>();
 
-  //  @Autowired
-  //  private ApplicationContext context;
-
   private IErnbClient ernbClient;
   private IZmrClient zmrClient;
 
@@ -94,14 +90,11 @@ public class InitialSearchTask extends AbstractAuthServletTask {
       final ILightResponse eidasResponse = authProcessData
           .getGenericDataFromSession(Constants.DATA_FULL_EIDAS_RESPONSE, ILightResponse.class);
 
-      // NOTE: Why is eidas first converted to a map, and then to a SimpleEidasData?
-      final Map<String, Object> simpleAttrMap = convertEidasAttrToSimpleMap(
-          eidasResponse.getAttributes().getAttributeMap());
-
       // post-process eIDAS attributes
-      final SimpleEidasData eidData = convertSimpleMapToSimpleData(simpleAttrMap);
+      final SimpleEidasData eidData = convertSimpleMapToSimpleData(convertEidasAttrToSimpleMap(
+          eidasResponse.getAttributes().getAttributeMap()));
 
-      String bpK = step2(eidData);
+      String bpK = step2RegisterSearchWithPersonidentifier(eidData);
       authProcessData.setGenericDataToSession(Constants.DATA_RESULT_MATCHING_BPK, bpK);
     } catch (final Exception e) {
       log.error("Initial search FAILED.", e);
@@ -109,22 +102,16 @@ public class InitialSearchTask extends AbstractAuthServletTask {
     }
   }
 
-  // NOTE: Please rename methods ... "step2" doesn't tell the reader anything
-  private String step2(SimpleEidasData eidData) throws TaskExecutionException {
+  private String step2RegisterSearchWithPersonidentifier(SimpleEidasData eidData) throws TaskExecutionException {
     String personIdentifier = eidData.getPseudonym();
-    // NOTE: Is that comment really necessary?
-    //search in register(step 2)
     MergedRegisterSearchResult result = searchInZmrAndErnp(personIdentifier);
     if (result.getResultCount() == 0) {
-      return step5(result, eidData);
+      return step5CheckCountrySpecificSearchPossible(result, eidData);
     } else if (result.getResultCount() == 1) {
-      return step3(result, eidData);
-      // NOTE: Why is that code commented-out?
-    } //else if (result.getResultCount() > 1) {
+      return step3CheckRegisterUpdateNecessary(result, eidData);
+    }
     throw new TaskExecutionException(pendingReq, "Initial search - Kitt Process necessary.",
         new ManualFixNecessaryException(personIdentifier));
-    //    }
-    //    return null;
   }
 
   private SimpleEidasData convertSimpleMapToSimpleData(Map<String, Object> eidasAttrMap)
@@ -163,33 +150,31 @@ public class InitialSearchTask extends AbstractAuthServletTask {
     return simpleEidasData;
   }
 
-  private String step3(MergedRegisterSearchResult result, SimpleEidasData eidData) throws TaskExecutionException {
+  private String step3CheckRegisterUpdateNecessary(MergedRegisterSearchResult result, SimpleEidasData eidData)
+      throws TaskExecutionException {
     //check if data from eidas authentication matches with data from register
-
     log.debug("Compare " + result + " with " + eidData);
-    //TODO check if data matches
     try {
       if (eidData.equalsRegisterData(result)) {
-        //TODO
+        //No update necessary, just return bpk
         return result.getBpk();
       } else {
-        return step4(result, eidData);
+        return step4UpdateRegisterData(result, eidData);
       }
     } catch (WorkflowException e) {
       throw new TaskExecutionException(pendingReq, "Initial search - Kitt Process necessary.", e);
     }
   }
 
-  private String step4(MergedRegisterSearchResult result,
-                       SimpleEidasData eidData) throws WorkflowException {
+  private String step4UpdateRegisterData(MergedRegisterSearchResult result,
+                                         SimpleEidasData eidData) throws WorkflowException {
     log.debug("Update " + result + " with " + eidData);
-    //TODO
+    //TODO wann rechtlich möglich?
 
-    // NOTE: Sometimes the bpk is returned, sometimes "105"?
     return result.getBpk();
   }
 
-  private String step5(MergedRegisterSearchResult result, SimpleEidasData eidData)
+  private String step5CheckCountrySpecificSearchPossible(MergedRegisterSearchResult result, SimpleEidasData eidData)
       throws TaskExecutionException {
     String citizenCountry = eidData.getCitizenCountryCode();
     ICountrySpecificDetailSearchProcessor foundHandler = null;
@@ -204,15 +189,15 @@ public class InitialSearchTask extends AbstractAuthServletTask {
     }
     if (foundHandler == null) {
       //MDS search
-      return step8(result, eidData);
+      return step8RegisterSearchWithMds(result, eidData);
     } else {
       //country specific search
-      return step6(foundHandler, result, eidData);
+      return step6CountrySpecificSearch(foundHandler, result, eidData);
     }
   }
 
-  private String step6(ICountrySpecificDetailSearchProcessor countrySpecificDetailSearchProcessor,
-                       MergedRegisterSearchResult initialSearchResult, SimpleEidasData eidData)
+  private String step6CountrySpecificSearch(ICountrySpecificDetailSearchProcessor countrySpecificDetailSearchProcessor,
+                                            MergedRegisterSearchResult initialSearchResult, SimpleEidasData eidData)
       throws TaskExecutionException {
     //6 country specific search
     MergedRegisterSearchResult countrySpecificDetailSearchResult =
@@ -220,19 +205,19 @@ public class InitialSearchTask extends AbstractAuthServletTask {
 
     switch (countrySpecificDetailSearchResult.getResultCount()) {
       case 0:
-        return step8(initialSearchResult, eidData);
+        return step8RegisterSearchWithMds(initialSearchResult, eidData);
       case 1:
-        return step7a(initialSearchResult, countrySpecificDetailSearchResult, eidData);
+        return step7aKittProcess(initialSearchResult, countrySpecificDetailSearchResult, eidData);
       default://should not happen
         throw new TaskExecutionException(pendingReq, "Detail search - Kitt Process necessary.",
             new ManualFixNecessaryException(eidData));
     }
   }
 
-  private String step7a(MergedRegisterSearchResult initialSearchResult,
-                        MergedRegisterSearchResult countrySpecificDetailSearchResult,
-                        SimpleEidasData eidData) throws TaskExecutionException {
-    //TODO automerge
+  private String step7aKittProcess(MergedRegisterSearchResult initialSearchResult,
+                                   MergedRegisterSearchResult countrySpecificDetailSearchResult,
+                                   SimpleEidasData eidData) throws TaskExecutionException {
+    //Automerge data
     log.debug("Automerge " + initialSearchResult + " with " + eidData + " " + countrySpecificDetailSearchResult);
     try {
       if (initialSearchResult.getResultCount() != 0) {
@@ -257,8 +242,8 @@ public class InitialSearchTask extends AbstractAuthServletTask {
     }
   }
 
-  private String step8(MergedRegisterSearchResult initialSearchResult,
-                       SimpleEidasData eidData) {
+  private String step8RegisterSearchWithMds(MergedRegisterSearchResult initialSearchResult,
+                                            SimpleEidasData eidData) {
     MergedRegisterSearchResult mdsSearchResult = new MergedRegisterSearchResult();
 
     ArrayList<RegisterResult> resultsZmr =
@@ -270,9 +255,8 @@ public class InitialSearchTask extends AbstractAuthServletTask {
     mdsSearchResult.setResultsErnb(resultsErnb);
 
     log.debug("Automerge " + initialSearchResult + " with " + eidData + " " + mdsSearchResult);
-    //TODO
-    // NOTE: Sometimes the bpk is returned, sometimes "105"?
-    return "105";
+    //TODO implement next phase and return correct value
+    return "TODO-Temporary-Endnode-105";
   }
 
   private MergedRegisterSearchResult searchInZmrAndErnp(String personIdentifier) {
@@ -334,24 +318,17 @@ public class InitialSearchTask extends AbstractAuthServletTask {
     return result;
   }
 
-  //just for testing
-  //TODO is there a nicer solution?
-  // There is: Constructor Injection see https://reflectoring.io/constructor-injection/ or https://www.baeldung.com/constructor-injection-in-spring
-  @Autowired
-  public void setErnbClient(IErnbClient ernbClient) {
+  /**
+   * Constructor.
+   * @param handlers List of countrySpecificSearchProcessors
+   * @param ernbClient Ernb client
+   * @param zmrClient ZMR client
+   */
+  public InitialSearchTask(List<ICountrySpecificDetailSearchProcessor> handlers, IErnbClient ernbClient,
+                           IZmrClient zmrClient) {
     this.ernbClient = ernbClient;
-  }
-
-  @Autowired
-  public void setZmrClient(IZmrClient zmrClient) {
     this.zmrClient = zmrClient;
-  }
-
-  @Autowired
-  public void setHandlers(List<ICountrySpecificDetailSearchProcessor> handlers) {
     this.handlers = handlers;
-    // NOTE: There's a typo in "registrated"
-    log.info("# " + handlers.size() + " country specific detail search services are registrated");
+    log.info("# " + handlers.size() + " country specific detail search services are registered");
   }
-
 }
diff --git a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/zmr/DummyZmrClient.java b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/zmr/DummyZmrClient.java
index 3af2e39e..f4d77b03 100644
--- a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/zmr/DummyZmrClient.java
+++ b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/zmr/DummyZmrClient.java
@@ -11,16 +11,7 @@ public class DummyZmrClient implements IZmrClient {
 
   @Override
   public ArrayList<RegisterResult> searchWithPersonIdentifer(String personIdentifer) {
-    // NOTE: Are we using Mockito or these fixed strings for testing, why are those defined here?
-    switch (personIdentifer) {
-      case "a12345":
-      case "a12345-":
-        return result1();
-      case "a123456":
-        return result2();
-      default:
-        return resultEmpty();
-    }
+    return resultEmpty();
   }
 
   @Override
@@ -48,20 +39,4 @@ public class DummyZmrClient implements IZmrClient {
     return new ArrayList<RegisterResult>();//Nobody found
   }
 
-  private ArrayList<RegisterResult> result1() {
-    ArrayList<RegisterResult> results = new ArrayList<>();
-    RegisterResult result1 = new RegisterResult("12345", "Tom", "Mustermann", "1950-01-01", "Wien");
-    results.add(result1);
-    RegisterResult result2 = new RegisterResult("12345-", "Tom", "Mustermann", "1950-01-01", "Wien");
-    results.add(result2);
-    return results;
-  }
-
-  private ArrayList<RegisterResult> result2() {
-    ArrayList<RegisterResult> results = new ArrayList<>();
-    RegisterResult result = new RegisterResult("123456", "Max", "Mustermann", "2000-01-01", "Wien");
-    results.add(result);
-    return results;
-  }
-
 }