[GitHub] [openmeetings] solomax commented on a change in pull request #166: OPENMEETINGS-2692 Update review comments on parsing path element.

[GitBox]

solomax commented on a change in pull request #166:
URL: https://github.com/apache/openmeetings/pull/166#discussion_r732013739



##########
File path: 
openmeetings-util/src/main/java/org/apache/openmeetings/util/OpenmeetingsVariables.java
##########
@@ -21,9 +21,16 @@
 import static org.apache.wicket.csp.CSPDirectiveSrcValue.SELF;
 import static org.apache.wicket.csp.CSPDirectiveSrcValue.STRICT_DYNAMIC;
 
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.github.openjson.JSONObject;
 
 public class OpenmeetingsVariables {
+       private static final Logger log = 
LoggerFactory.getLogger(OpenmeetingsVariables.class);

Review comment:
       this can be dropped

##########
File path: 
openmeetings-util/src/main/java/org/apache/openmeetings/util/OpenmeetingsVariables.java
##########
@@ -249,11 +256,12 @@ public static String getBaseUrl() {
        }
 
        public static String getWebappPath() {
-               String webappPath = baseUrl;
-               if (webappPath.endsWith("/")) {
-                       webappPath = webappPath.substring(0, 
webappPath.length() - 1);
+               try {

Review comment:
       Sorry for the late response
   Tuesday is a bit crazy day :(((
   
   I've just played with `URI`s in `jshell`
   and here is what I propose :)))
   
   This method can be rewritten as
   ```
   public static URI getWebappPath() {
       return URI.create(URI.create(baseUrl + "/").normalize().getPath());
   }
   ```
   
   this way the `URI` returned will always have trailing '/'

##########
File path: 
openmeetings-webservice/src/main/java/org/apache/openmeetings/webservice/InfoWebService.java
##########
@@ -106,7 +106,7 @@ public String getManifest() {
                manifest.put("name", OpenmeetingsVariables.getApplicationName() 
+ " " + Version.getVersion());
                manifest.put("short_name", 
OpenmeetingsVariables.getApplicationName() + " " + Version.getVersion());
                manifest.put("description", "Openmeetings provides video 
conferencing, instant messaging, white board, collaborative document editing 
and other groupware tools.");
-               manifest.put("start_url", "/" + 
OpenmeetingsVariables.getWebappPath() + "/?pwa=true");
+               manifest.put("start_url",  
OpenmeetingsVariables.getWebappPath() + "?pwa=true");

Review comment:
       here you can do something like:
   
   ```
   URI omPath = OpenmeetingsVariables.getWebappPath(); // to avoid 
recalculations
   
   then
   
   omPath.resolve("?pwa=true")
   
   omPath.resolve("images/icons/" + name)
   OR 
   omPath.resolve("images/icons/").resolve(name)
   ```




-- 
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: dev-unsubscr...@openmeetings.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org