chibenwa commented on a change in pull request #780:
URL: https://github.com/apache/james-project/pull/780#discussion_r763082906



##########
File path: 
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/JmapRfc8621Configuration.scala
##########
@@ -23,45 +23,65 @@ import java.net.{URI, URL}
 import java.util.Optional
 
 import org.apache.commons.configuration2.Configuration
-import org.apache.james.jmap.core.JmapRfc8621Configuration.UPLOAD_LIMIT_30_MB
+import org.apache.james.jmap.core.JmapRfc8621Configuration.UPLOAD_LIMIT_DEFAULT
 import org.apache.james.jmap.pushsubscription.PushClientConfiguration
 import org.apache.james.util.Size
 
 import scala.jdk.OptionConverters._
 
+object JmapConfigProperties {
+  val UPLOAD_LIMIT_PROPERTY: String = "upload.max.size"
+  val URL_PREFIX_PROPERTY: String = "url.prefix"
+  val WEBSOCKET_URL_PREFIX_PROPERTY: String = "websocket.url.prefix"
+  val WEB_PUSH_MAX_TIMEOUT_SECONDS_PROPERTY: String = 
"webpush.maxTimeoutSeconds"
+  val WEB_PUSH_MAX_CONNECTIONS_PROPERTY: String = "webpush.maxConnections"
+  val DYNAMIC_JMAP_PREFIX_RESOLUTION_ENABLED_PROPERTY: String = 
"dynamic.jmap.prefix.resolution.enabled"
+}
+
 object JmapRfc8621Configuration {
-  val LOCALHOST_URL_PREFIX: String = "http://localhost";
-  val LOCALHOST_WEBSOCKET_URL_PREFIX: String = "ws://localhost"
-  val UPLOAD_LIMIT_30_MB: MaxSizeUpload = MaxSizeUpload.of(Size.of(30L, 
Size.Unit.M)).get
-  val LOCALHOST_CONFIGURATION: JmapRfc8621Configuration = 
JmapRfc8621Configuration(LOCALHOST_URL_PREFIX, LOCALHOST_WEBSOCKET_URL_PREFIX, 
UPLOAD_LIMIT_30_MB)
-  val URL_PREFIX_PROPERTIES: String = "url.prefix"
-  val WEBSOCKET_URL_PREFIX_PROPERTIES: String = "websocket.url.prefix"
-  val UPLOAD_LIMIT_PROPERTIES: String = "upload.max.size"
+  import JmapConfigProperties._
+  val URL_PREFIX_DEFAULT: String = "http://localhost";
+  val WEBSOCKET_URL_PREFIX_DEFAULT: String = "ws://localhost"
+  val UPLOAD_LIMIT_DEFAULT: MaxSizeUpload = MaxSizeUpload.of(Size.of(30L, 
Size.Unit.M)).get
+
+  val LOCALHOST_CONFIGURATION: JmapRfc8621Configuration = 
JmapRfc8621Configuration(
+    urlPrefixString = URL_PREFIX_DEFAULT,
+    websocketPrefixString = WEBSOCKET_URL_PREFIX_DEFAULT,
+    maxUploadSize = UPLOAD_LIMIT_DEFAULT)
 
-  def from(configuration: Configuration): JmapRfc8621Configuration = {
+  def from(configuration: Configuration): JmapRfc8621Configuration =
     JmapRfc8621Configuration(
-      urlPrefixString = 
Option(configuration.getString(URL_PREFIX_PROPERTIES)).getOrElse(LOCALHOST_URL_PREFIX),
-      websocketPrefixString = 
Option(configuration.getString(WEBSOCKET_URL_PREFIX_PROPERTIES)).getOrElse(LOCALHOST_WEBSOCKET_URL_PREFIX),
-      maxUploadSize = Option(configuration.getString(UPLOAD_LIMIT_PROPERTIES, 
null))
+      urlPrefixString = 
Option(configuration.getString(URL_PREFIX_PROPERTY)).getOrElse(URL_PREFIX_DEFAULT),
+      websocketPrefixString = 
Option(configuration.getString(WEBSOCKET_URL_PREFIX_PROPERTY)).getOrElse(WEBSOCKET_URL_PREFIX_DEFAULT),
+      dynamicJmapPrefixResolutionEnabled = 
configuration.getBoolean(DYNAMIC_JMAP_PREFIX_RESOLUTION_ENABLED_PROPERTY, 
false),
+      maxUploadSize = Option(configuration.getString(UPLOAD_LIMIT_PROPERTY, 
null))
         .map(Size.parse)
         .map(MaxSizeUpload.of(_).get)
-        .getOrElse(UPLOAD_LIMIT_30_MB),
-      maxTimeoutSeconds = 
Optional.ofNullable(configuration.getInteger("webpush.maxTimeoutSeconds", 
null)).map(Integer2int).toScala,
-      maxConnections = 
Optional.ofNullable(configuration.getInteger("webpush.maxConnections", 
null)).map(Integer2int).toScala)
-  }
+        .getOrElse(UPLOAD_LIMIT_DEFAULT),
+      maxTimeoutSeconds = 
Optional.ofNullable(configuration.getInteger(WEB_PUSH_MAX_TIMEOUT_SECONDS_PROPERTY,
 null)).map(Integer2int).toScala,
+      maxConnections = 
Optional.ofNullable(configuration.getInteger(WEB_PUSH_MAX_CONNECTIONS_PROPERTY, 
null)).map(Integer2int).toScala)
 }
 
-case class JmapRfc8621Configuration(urlPrefixString: String, 
websocketPrefixString: String,
-                                    maxUploadSize: MaxSizeUpload = 
UPLOAD_LIMIT_30_MB,
+case class JmapRfc8621Configuration(urlPrefixString: String,
+                                    websocketPrefixString: String,
+                                    dynamicJmapPrefixResolutionEnabled: 
Boolean = false,
+                                    maxUploadSize: MaxSizeUpload = 
UPLOAD_LIMIT_DEFAULT,
                                     maxTimeoutSeconds: Option[Int] = None,
                                     maxConnections: Option[Int] = None) {
-  val urlPrefix: URL = new URL(urlPrefixString)
-  val apiUrl: URL = new URL(s"$urlPrefixString/jmap")
-  val downloadUrl: URL = new URL(urlPrefixString + 
"/download/{accountId}/{blobId}?type={type}&name={name}")
-  val uploadUrl: URL = new URL(s"$urlPrefixString/upload/{accountId}")
-  val eventSourceUrl: URL = new 
URL(s"$urlPrefixString/eventSource?types={types}&closeAfter={closeafter}&ping={ping}")
-  val webSocketUrl: URI = new URI(s"$websocketPrefixString/jmap/ws")
 
+  def getURLPrefix(urlPrefixRequest: Option[URL]): String = 
Some(dynamicJmapPrefixResolutionEnabled)

Review comment:
       What is that `Option[URL]`?
   
   Is it the role of the configuration POJO to "resolve the URL"? Or should it 
be done somewhere else by another abstraction?
   
   Can't the POJO just pass the configured prefix to that other entity if any?

##########
File path: 
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/core/JmapRfc8621Configuration.scala
##########
@@ -23,45 +23,65 @@ import java.net.{URI, URL}
 import java.util.Optional
 
 import org.apache.commons.configuration2.Configuration
-import org.apache.james.jmap.core.JmapRfc8621Configuration.UPLOAD_LIMIT_30_MB
+import org.apache.james.jmap.core.JmapRfc8621Configuration.UPLOAD_LIMIT_DEFAULT
 import org.apache.james.jmap.pushsubscription.PushClientConfiguration
 import org.apache.james.util.Size
 
 import scala.jdk.OptionConverters._
 
+object JmapConfigProperties {
+  val UPLOAD_LIMIT_PROPERTY: String = "upload.max.size"
+  val URL_PREFIX_PROPERTY: String = "url.prefix"
+  val WEBSOCKET_URL_PREFIX_PROPERTY: String = "websocket.url.prefix"
+  val WEB_PUSH_MAX_TIMEOUT_SECONDS_PROPERTY: String = 
"webpush.maxTimeoutSeconds"
+  val WEB_PUSH_MAX_CONNECTIONS_PROPERTY: String = "webpush.maxConnections"
+  val DYNAMIC_JMAP_PREFIX_RESOLUTION_ENABLED_PROPERTY: String = 
"dynamic.jmap.prefix.resolution.enabled"
+}
+
 object JmapRfc8621Configuration {
-  val LOCALHOST_URL_PREFIX: String = "http://localhost";
-  val LOCALHOST_WEBSOCKET_URL_PREFIX: String = "ws://localhost"
-  val UPLOAD_LIMIT_30_MB: MaxSizeUpload = MaxSizeUpload.of(Size.of(30L, 
Size.Unit.M)).get
-  val LOCALHOST_CONFIGURATION: JmapRfc8621Configuration = 
JmapRfc8621Configuration(LOCALHOST_URL_PREFIX, LOCALHOST_WEBSOCKET_URL_PREFIX, 
UPLOAD_LIMIT_30_MB)
-  val URL_PREFIX_PROPERTIES: String = "url.prefix"
-  val WEBSOCKET_URL_PREFIX_PROPERTIES: String = "websocket.url.prefix"
-  val UPLOAD_LIMIT_PROPERTIES: String = "upload.max.size"
+  import JmapConfigProperties._
+  val URL_PREFIX_DEFAULT: String = "http://localhost";
+  val WEBSOCKET_URL_PREFIX_DEFAULT: String = "ws://localhost"
+  val UPLOAD_LIMIT_DEFAULT: MaxSizeUpload = MaxSizeUpload.of(Size.of(30L, 
Size.Unit.M)).get
+
+  val LOCALHOST_CONFIGURATION: JmapRfc8621Configuration = 
JmapRfc8621Configuration(
+    urlPrefixString = URL_PREFIX_DEFAULT,
+    websocketPrefixString = WEBSOCKET_URL_PREFIX_DEFAULT,
+    maxUploadSize = UPLOAD_LIMIT_DEFAULT)
 
-  def from(configuration: Configuration): JmapRfc8621Configuration = {
+  def from(configuration: Configuration): JmapRfc8621Configuration =
     JmapRfc8621Configuration(
-      urlPrefixString = 
Option(configuration.getString(URL_PREFIX_PROPERTIES)).getOrElse(LOCALHOST_URL_PREFIX),
-      websocketPrefixString = 
Option(configuration.getString(WEBSOCKET_URL_PREFIX_PROPERTIES)).getOrElse(LOCALHOST_WEBSOCKET_URL_PREFIX),
-      maxUploadSize = Option(configuration.getString(UPLOAD_LIMIT_PROPERTIES, 
null))
+      urlPrefixString = 
Option(configuration.getString(URL_PREFIX_PROPERTY)).getOrElse(URL_PREFIX_DEFAULT),
+      websocketPrefixString = 
Option(configuration.getString(WEBSOCKET_URL_PREFIX_PROPERTY)).getOrElse(WEBSOCKET_URL_PREFIX_DEFAULT),
+      dynamicJmapPrefixResolutionEnabled = 
configuration.getBoolean(DYNAMIC_JMAP_PREFIX_RESOLUTION_ENABLED_PROPERTY, 
false),
+      maxUploadSize = Option(configuration.getString(UPLOAD_LIMIT_PROPERTY, 
null))
         .map(Size.parse)
         .map(MaxSizeUpload.of(_).get)
-        .getOrElse(UPLOAD_LIMIT_30_MB),
-      maxTimeoutSeconds = 
Optional.ofNullable(configuration.getInteger("webpush.maxTimeoutSeconds", 
null)).map(Integer2int).toScala,
-      maxConnections = 
Optional.ofNullable(configuration.getInteger("webpush.maxConnections", 
null)).map(Integer2int).toScala)
-  }
+        .getOrElse(UPLOAD_LIMIT_DEFAULT),
+      maxTimeoutSeconds = 
Optional.ofNullable(configuration.getInteger(WEB_PUSH_MAX_TIMEOUT_SECONDS_PROPERTY,
 null)).map(Integer2int).toScala,
+      maxConnections = 
Optional.ofNullable(configuration.getInteger(WEB_PUSH_MAX_CONNECTIONS_PROPERTY, 
null)).map(Integer2int).toScala)
 }
 
-case class JmapRfc8621Configuration(urlPrefixString: String, 
websocketPrefixString: String,
-                                    maxUploadSize: MaxSizeUpload = 
UPLOAD_LIMIT_30_MB,
+case class JmapRfc8621Configuration(urlPrefixString: String,
+                                    websocketPrefixString: String,
+                                    dynamicJmapPrefixResolutionEnabled: 
Boolean = false,
+                                    maxUploadSize: MaxSizeUpload = 
UPLOAD_LIMIT_DEFAULT,
                                     maxTimeoutSeconds: Option[Int] = None,
                                     maxConnections: Option[Int] = None) {
-  val urlPrefix: URL = new URL(urlPrefixString)
-  val apiUrl: URL = new URL(s"$urlPrefixString/jmap")
-  val downloadUrl: URL = new URL(urlPrefixString + 
"/download/{accountId}/{blobId}?type={type}&name={name}")
-  val uploadUrl: URL = new URL(s"$urlPrefixString/upload/{accountId}")
-  val eventSourceUrl: URL = new 
URL(s"$urlPrefixString/eventSource?types={types}&closeAfter={closeafter}&ping={ping}")
-  val webSocketUrl: URI = new URI(s"$websocketPrefixString/jmap/ws")
 
+  def getURLPrefix(urlPrefixRequest: Option[URL]): String = 
Some(dynamicJmapPrefixResolutionEnabled)
+    .filter(enabled => enabled)
+    .flatMap(_ => urlPrefixRequest.map(_.toString))
+    .getOrElse(urlPrefixString)
+
+  def apiUrl(urlPrefixRequest: Option[URL] ): URL = new 
URL(s"${getURLPrefix(urlPrefixRequest)}/jmap")
+
+  def downloadUrl(urlPrefixRequest: Option[URL]): URL = new 
URL(s"${getURLPrefix(urlPrefixRequest)}/download/{accountId}/{blobId}?type={type}&name={name}")
+
+  def uploadUrl(urlPrefixRequest: Option[URL]): URL = new 
URL(s"${getURLPrefix(urlPrefixRequest)}/upload/{accountId}")
+
+  val webSocketUrl: URI = new URI(s"$websocketPrefixString/jmap/ws")
+  val eventSourceUrl: URL = new 
URL(s"$urlPrefixString/eventSource?types={types}&closeAfter={closeafter}&ping={ping}")

Review comment:
       Is a configuration POJO the right place for advanced resolution? 
Shouldn't an other entity do this logic relying only on the prefix shiped by 
the configuration?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to