Problem details refactor (#1487)

* feat(interceptor): print plugin for logging messages

* feat(interceptor): print plugin for logging messages

* refactor(soap):

Fixed deprecation

* docs: roadmap

* test:  fixes

* refactor(ProblemDetails):

minor

* refactor(ProblemDetails):

minor

core/src/main/java/com/predic8/membrane/core/exceptions/ProblemDetails.java

@@ -36,9 +36,9 @@
 public class ProblemDetails {
 
     private static final Logger log = LoggerFactory.getLogger(ProblemDetails.class.getName());
-  
+
     private static final ObjectMapper om = new ObjectMapper();
-    private final static ObjectWriter ow = om.writerWithDefaultPrettyPrinter();
+    private static final ObjectWriter ow = om.writerWithDefaultPrettyPrinter();
 
     private boolean production;
 
@@ -49,10 +49,10 @@ public class ProblemDetails {
     private String detail;
 
     /**
-     * Component like plugin
+     * Component e.g. plugin
      */
     private String component;
-    
+
     private String instance;
     private final HashMap<String, Object> extensions = new LinkedHashMap<>();
     private Throwable exception;
@@ -71,11 +71,11 @@ public static ProblemDetails internal(boolean production) {
     }
 
     public static ProblemDetails gateway(boolean production) {
-        return problemDetails( "gateway", production).statusCode(500).title("Gateway error.");
+        return problemDetails("gateway", production).statusCode(500).title("Gateway error.");
     }
 
     public static ProblemDetails security(boolean production) {
-        return problemDetails( "security", production);
+        return problemDetails("security", production);
     }
 
     public static ProblemDetails openapi(boolean production) {
@@ -89,6 +89,12 @@ public static ProblemDetails problemDetails(String type, boolean production) {
         return pd;
     }
 
+    /**
+     * type/subtype/subtype/...
+     * lowercase, dash as separator
+     * @param subType
+     * @return
+     */
     public ProblemDetails addSubType(String subType) {
         this.type += "/" + subType;
         return this;
@@ -114,7 +120,7 @@ public ProblemDetails detail(String humanReadableExplanation) {
             this.detail = humanReadableExplanation;
         return this;
     }
-    
+
     public ProblemDetails component(String component) {
         this.component = component;
         return this;
@@ -146,7 +152,7 @@ public Response build() {
 
     /**
      * Does only log, when key in log is needed. The caller is responsible to do the log if
-     * there is something interessting.
+     * there is something interesting.
      */
     public void buildAndSetResponse(Exchange exchange) {
         if (exchange != null) {
@@ -186,13 +192,13 @@ private void logDevelopment(Map<String, Object> extensionsMap) {
         if (exception != null) {
             if (extensionsMap.containsKey("message"))
                 log.error("Overriding ProblemDetails extensionsMap 'message' entry. Please notify Membrane developers.", new RuntimeException());
-            extensionsMap.put("message",exception.getMessage());
+            extensionsMap.put("message", exception.getMessage());
             if (stacktrace) {
                 extensionsMap.put("stackTrace", getStackTrace());
             }
         }
         extensionsMap.put("attention", """
-            Membrane is in development mode. For production set <router production="true"> to reduce details in error messages!""");
+                Membrane is in development mode. For production set <router production="true"> to reduce details in error messages!""");
     }
 
     private void logProduction(Map<String, Object> extensionsMap) {
@@ -203,15 +209,15 @@ private void logProduction(Map<String, Object> extensionsMap) {
         title = "An error occurred.";
         detail = "Details can be found in the Membrane log searching for key: %s.".formatted(logKey);
         if (stacktrace) {
-            log.warn("",exception);
+            log.warn("", exception);
         }
     }
 
     private @NotNull Map getStackTrace() {
         var m = new LinkedHashMap<>();
-            for (int i = 0; i < exception.getStackTrace().length; i++) {
-                m.put("e"+i , exception.getStackTrace()[i].toString());
-            }
+        for (int i = 0; i < exception.getStackTrace().length; i++) {
+            m.put("e" + i, exception.getStackTrace()[i].toString());
+        }
         return m;
     }
 
@@ -278,15 +284,16 @@ private static void createJson(Map<String, Object> root, Response.ResponseBuilde
     public static ProblemDetails parse(Response r) throws JsonProcessingException {
 
         if (r.getHeader().getContentType() == null)
-            throw  new RuntimeException("No Content-Type in message with ProblemDetails!");
+            throw new RuntimeException("No Content-Type in message with ProblemDetails!");
 
         if (!r.getHeader().getContentType().equals(APPLICATION_PROBLEM_JSON))
-            throw new RuntimeException("Content-Type ist %s but should be %s.".formatted(r.getHeader().getContentType(),APPLICATION_PROBLEM_JSON));
+            throw new RuntimeException("Content-Type ist %s but should be %s.".formatted(r.getHeader().getContentType(), APPLICATION_PROBLEM_JSON));
 
         ProblemDetails pd = new ProblemDetails();
         pd.statusCode(r.getStatusCode());
 
-        TypeReference<Map<String,Object>> typeRef = new TypeReference<>() {};
+        TypeReference<Map<String, Object>> typeRef = new TypeReference<>() {
+        };
 
         Map<String, Object> m = om.readValue(r.getBodyAsStringDecoded(), typeRef);
 
@@ -295,42 +302,46 @@ public static ProblemDetails parse(Response r) throws JsonProcessingException {
         pd.detail = (String) m.get("detail");
         pd.instance = (String) m.get("instance");
 
-        for (Map.Entry<String, Object> e :m.entrySet()) {
-            if(pd.isReservedProblemDetailsField(e.getKey()))
+        for (Map.Entry<String, Object> e : m.entrySet()) {
+            if (pd.isReservedProblemDetailsField(e.getKey()))
                 continue;
-            pd.extension(e.getKey(),e.getValue());
+            pd.extension(e.getKey(), e.getValue());
         }
         return pd;
     }
 
     private boolean isReservedProblemDetailsField(String key) {
-        for (String reserved : List.of("type","title","detail","instance")) {
+        for (String reserved : List.of("type", "title", "detail", "instance")) {
             if (key.equals(reserved))
                 return true;
         }
         return false;
     }
 
-    public boolean isProduction() {
-        return production;
-    }
-
-    public int getStatusCode() {
-        return statusCode;
+    public String getTitle() {
+        return title;
     }
 
     public String getType() {
         return type;
     }
 
-    public String getTitle() {
-        return title;
+    public int getStatusCode() {
+        return statusCode;
+    }
+
+    public boolean isProduction() {
+        return production;
     }
 
     public String getDetail() {
         return detail;
     }
 
+    public String getComponent() {
+        return component;
+    }
+
     public String getInstance() {
         return instance;
     }
@@ -343,6 +354,10 @@ public Throwable getException() {
         return exception;
     }
 
+    public boolean isStacktrace() {
+        return stacktrace;
+    }
+
     @Override
     public String toString() {
         return "ProblemDetails{" +

core/src/main/java/com/predic8/membrane/core/exchange/Exchange.java

@@ -33,7 +33,7 @@ public class Exchange extends AbstractExchange {
 
 	public static final String /*PROPERTY_*/ALLOW_TCP = "use-tcp";
 
-	public static final String /*PROPERTY_*/ALLOW_SPDY = "use-sdpy";
+	public static final String /*PROPERTY_*/ALLOW_SPDY = "use-sdpy"; // Remove
 
 	public static final String /*PROPERTY_*/TRACK_NODE_STATUS = "TRACK_NODE_STATUS";
 

core/src/main/java/com/predic8/membrane/core/interceptor/LoggingContextInterceptor.java

@@ -19,6 +19,9 @@
 
 import static com.predic8.membrane.core.interceptor.Outcome.CONTINUE;
 
+/**
+ * Adds the proxy name to log lines
+ */
 @MCElement(name="logContext")
 public class LoggingContextInterceptor extends AbstractInterceptor{
     private final String proxyName = "proxyName";

core/src/main/java/com/predic8/membrane/core/interceptor/javascript/JavascriptInterceptor.java

@@ -70,15 +70,9 @@ protected void handleScriptExecutionException(Exchange exc, Exception e) {
         log.warn("Script: {}", src);
 
         ProblemDetails pd = adapter.getProblemDetails(e);
-
         pd.title("Error executing script.");
-
-        if (!router.isProduction()) {
-            pd.extension("message", e.getMessage())
-                    .extension("source", trim(src));
-        } else {
-            pd.detail("See logs for details.");
-        }
+        pd.extension("message", e.getMessage())
+                .extension("source", trim(src));
 
         exc.setResponse(pd.build());
     }

core/src/main/java/com/predic8/membrane/core/interceptor/javascript/LanguageAdapter.java

@@ -28,7 +28,7 @@
 
 public abstract  class LanguageAdapter {
 
-    private static final Logger log = LoggerFactory.getLogger(JavascriptInterceptor.class);
+    private static final Logger log = LoggerFactory.getLogger(LanguageAdapter.class);
 
     protected LanguageSupport languageSupport;
     final protected Router router;

core/src/main/java/com/predic8/membrane/core/interceptor/jwt/JwtSignInterceptor.java

@@ -32,6 +32,7 @@
 import static com.predic8.membrane.core.interceptor.Interceptor.Flow.*;
 import static com.predic8.membrane.core.interceptor.Outcome.ABORT;
 import static com.predic8.membrane.core.interceptor.Outcome.*;
+import static org.jose4j.jws.AlgorithmIdentifiers.RSA_USING_SHA256;
 
 @MCElement(name = "jwtSign")
 public class JwtSignInterceptor extends AbstractInterceptor {
@@ -71,25 +72,25 @@ private Outcome handleInternal(Exchange exc, Flow flow) {
             JsonWebSignature jws = new JsonWebSignature();
             jws.setPayload(prepareJwtPayload(exc.getMessage(flow)));
             jws.setKey(rsaJsonWebKey.getRsaPrivateKey());
-            jws.setAlgorithmHeaderValue(AlgorithmIdentifiers.RSA_USING_SHA256);
+            jws.setAlgorithmHeaderValue(RSA_USING_SHA256);
             jws.setKeyIdHeaderValue(rsaJsonWebKey.getKeyId());
             exc.getMessage(flow).setBodyContent(jws.getCompactSerialization().getBytes());
             return CONTINUE;
         } catch (JoseException e) {
             log.error("Error during attempt to sign JWT payload: {}", e.getLocalizedMessage());
-            ProblemDetails.gateway(router.isProduction())
+            ProblemDetails.security(router.isProduction())
+                    .addSubType("crypto")
                     .component(getDisplayName())
-                    .extension("message", e.getLocalizedMessage())
                     .detail("Error during attempt to sign JWT payload.")
                     .exception(e)
                     .stacktrace(true)
                     .buildAndSetResponse(exc);
             return ABORT;
         } catch (IOException e) {
             log.error("Error during attempt to parse JWT payload: {}", e.getLocalizedMessage());
-            ProblemDetails.gateway(router.isProduction())
+            ProblemDetails.security(router.isProduction())
+                    .addSubType("io")
                     .component(getDisplayName())
-                    .extension("message", e.getLocalizedMessage())
                     .detail("Error during attempt to parse JWT payload.")
                     .exception(e)
                     .stacktrace(true)

docs/ROADMAP.md

@@ -30,6 +30,13 @@
 - JdbcUserDataProvider
   - Migrate to PreparedStatement
 - <target url="http://localhost:2000/${params.product}/>
+- OAuth2 refactoring
+  - In Interface com.predic8.membrane.core.interceptor.oauth2.authorizationservice.AuthorizationService
+    - Remove throws:
+      - public abstract String getJwksEndpoint() throws Exception;
+      - public abstract void init() throws Exception;
+      - getEndSessionEndpoint() throws Exception
+      - doDynamicRegistration(List<String> callbackURLs) throws Exception
 
 # Version 6.0.0
 
@@ -55,40 +62,50 @@
 
 
 ### Internal
-- Rename service:// to internal://
 - Delete interceptor/
   - Gatekeeper: Yes
-  - SDY Speedy?
-  - JsonPointer Extractor?
-- proxies-2.xsd
-  - new Namespace e.g. https://membrane-api.io...
-- 2025
-- Test in proxies.xml internal with port
-- Interceptor init() and init(Router router)
-- ProblemDetails
+  - SDY Speedy (delete) BT
+- proxies-6.xsd
+  - new Namespace e.g. https://membrane-api.io...6
+- 2025 BT
+- ProblemDetails TB
   - Validators
      - XML, JSON, WSDL
      - All in examples/validation
   - OpenAPI
-- In com.predic8.membrane.core.interceptor.oauth2.authorizationservice.AuthorizationService 
-  - Remove throws: 
-    - public abstract String getJwksEndpoint() throws Exception; 
-- Exchange property name constants: See Exchange
-- Dependencies
+- Exchange property name constants: See Exchange TB
+- Logging TB
+  - Simple logger raus
+  - JSON logging raus
+- Dependencies TP, TB
   - Log4J, where, what
-- Interceptor Interface:
-  - handle should not throw Exception
-- AdminConsole
+  - Updates
+- AdminConsole BT
   - DisplayName: make all lower
-- Cleaup:
-  - LoggingContextInterceptor? Ask CG
-- Merge log with print
-  <log message="${header.foo}/>
-  default: message="${header}\n${body}"
-- Remove etcd stuff 
-- Rewrite RatelimitInterceptor to use AbstractLanguageInterceptor
+- LoggingContextInterceptor? Ask CG
+  - Is api name logged?
+  - Comment
+- Remove etcd stuff (BT)
+- Rewrite RatelimitInterceptor to use AbstractLanguageInterceptor TB
+- Language TB
+  - Exchange expression 
+    - getExpression
+  - AbstractLanguageInterceptor as Interface
+- Problem Details TB
+  - /type/component/subtype
+  - component mandatory, subtype optional
+  - Detail: 
+    - Is used everywhere (ADR)
+    - Human easy readable
+    - optional
+  - Message from exception in message
+    - Toplevel not in extension
+    - optional
+    - Message only from exception
+      - Disable in builder
 
 ## Done
+- Merge log with print
 - XMLProtectionInterceptor.setFailResponse => Use ProblemDetails
 - Rename ExampleTests to .*ExampleTests
 - Examples