Merge pull request #5700 from kwvanderlinde/bugfix/5696-macro-location

Fix macro callable locations

Author: kwvanderlinde

src/main/java/net/rptools/maptool/client/macro/MacroLocation.java

@@ -15,7 +15,6 @@
 package net.rptools.maptool.client.macro;
 
 import java.net.URI;
-import java.net.URISyntaxException;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import net.rptools.maptool.model.Token;
@@ -52,12 +51,12 @@ public class MacroLocation {
   /** The location of the macro. */
   @Nonnull private final String location;
 
+  /** the location of the macro that can be called from macro code using the '@' syntax */
+  @Nonnull private final String callableLocation;
+
   /** The URI of the macro, if applicable. */
   @Nullable private final URI uri;
 
-  /** The token associated with this macro, if applicable. */
-  @Nullable private final Token token;
-
   /** MacroLocationFactory used to create locations during parsing. */
   private static final MacroLocationFactory factory = MacroLocationFactory.getInstance();
 
@@ -68,19 +67,18 @@ public class MacroLocation {
    * @param source the source of the macro.
    * @param location the location of the macro.
    * @param uri the URI of the macro, if applicable.
-   * @param token the associated token, if applicable.
    */
   MacroLocation(
       @Nonnull String name,
       @Nonnull MacroSource source,
       @Nonnull String location,
-      @Nullable URI uri,
-      @Nullable Token token) {
+      @Nonnull String callableLocation,
+      @Nullable URI uri) {
     this.name = name;
     this.source = source;
     this.location = location;
+    this.callableLocation = callableLocation;
     this.uri = uri;
-    this.token = token;
   }
 
   /**
@@ -90,8 +88,12 @@ public class MacroLocation {
    * @param source the source of the macro.
    * @param location the location of the macro.
    */
-  MacroLocation(@Nonnull String name, @Nonnull MacroSource source, @Nonnull String location) {
-    this(name, source, location, null, null);
+  MacroLocation(
+      @Nonnull String name,
+      @Nonnull MacroSource source,
+      @Nonnull String location,
+      @Nonnull String callableLocation) {
+    this(name, source, location, callableLocation, null);
   }
 
   /** Enumeration to represent the source of the macro. */
@@ -173,42 +175,29 @@ public static MacroLocation parseMacroName(
     String qMacroNameLower = qMacroName.toLowerCase();
 
     if (qMacroNameLower.contains("@campaign")) {
-      return new MacroLocation(
-          qMacroName.substring(0, qMacroName.indexOf("@")),
-          MacroSource.campaign,
-          MacroSource.campaign.getSourceName());
+      return factory.createCampaignLocation(qMacroName.substring(0, qMacroName.indexOf("@")));
     }
 
     if (qMacroNameLower.contains("@gm")) {
-      return new MacroLocation(
-          qMacroName.substring(0, qMacroName.indexOf("@")),
-          MacroSource.gm,
-          MacroSource.gm.getSourceName());
+      return factory.createGmLocation(qMacroName.substring(0, qMacroName.indexOf("@")));
     }
 
     if (qMacroNameLower.contains("@global")) {
-      return new MacroLocation(
-          qMacroName.substring(0, qMacroName.indexOf("@")),
-          MacroSource.global,
-          MacroSource.global.getSourceName());
+      return factory.createGlobalLocation(qMacroName.substring(0, qMacroName.indexOf("@")));
     }
 
     if (qMacroNameLower.contains("@token")) {
       if (token == null) {
         return factory.createUnknownLocation(qMacroName);
       }
-      return new MacroLocation(
-          qMacroName.substring(0, qMacroName.indexOf("@")),
-          MacroSource.token,
-          token.getName(),
-          null,
-          token);
+      return factory.createTokenLocation(qMacroName.substring(0, qMacroName.indexOf("@")), token);
     }
 
     if (qMacroNameLower.contains("@lib:")) {
       String macroName = qMacroName.substring(0, qMacroName.indexOf("@"));
-      String namespace = qMacroName.replaceFirst("^[^@]*@(?i)lib:", "");
-      return new MacroLocation(macroName, MacroSource.library, namespace, null, token);
+      String location = qMacroName.substring(qMacroName.indexOf("@") + 1);
+      String namespace = location.replaceFirst("(?i)lib:", "");
+      return new MacroLocation(macroName, MacroSource.library, namespace, location);
     }
 
     if (qMacroNameLower.contains("@this")) {
@@ -222,30 +211,16 @@ public static MacroLocation parseMacroName(
           return factory.createUnknownLocation(qMacroName);
         }
       }
-      return new MacroLocation(name, cfrom.getSource(), cfrom.getLocation(), null, token);
+      return new MacroLocation(
+          name, cfrom.getSource(), cfrom.getLocation(), cfrom.getCallableLocation());
     }
 
     // If none of the above then assume it is a URI
-    URI uri;
-    try {
-      uri = new URI(qMacroName);
-    } catch (URISyntaxException e) {
-      return factory.createUnknownLocation(qMacroName);
-    }
-
-    if (uri.getHost() == null) {
-      if (calledFrom != null && calledFrom.getSource() == MacroSource.uri) {
-        uri = calledFrom.getUri().resolve(uri);
-      } else {
-        return factory.createUnknownLocation(qMacroName);
-      }
-    }
-
-    if (uri.getScheme() == null || !uri.getScheme().toLowerCase().equals("lib")) {
-      return factory.createUnknownLocation(qMacroName);
-    }
-
-    return new MacroLocation(uri.getPath().substring(1), MacroSource.uri, uri.getHost(), uri, null);
+    return factory.createUriLocation(
+        qMacroName,
+        calledFrom != null && calledFrom.getSource() == MacroSource.uri
+            ? calledFrom.getUri()
+            : null);
   }
 
   /**
@@ -284,29 +259,14 @@ public URI getUri() {
     return uri;
   }
 
-  /**
-   * Returns the token associated with this macro, if applicable.
-   *
-   * @return the token associated with this macro, or null if not applicable.
-   */
-  public Token getToken() {
-    return token;
-  }
-
   /**
    * Returns the location of the macro that can be called from macro code using the '@' syntax.
    *
    * @return the location of the macro that can be called from other macros.
    */
   @Nonnull
   public String getCallableLocation() {
-    if (source == MacroSource.token) {
-      return "Token:" + location;
-    } else if (source == MacroSource.library) {
-      return token != null ? token.getName() : location;
-    } else {
-      return location;
-    }
+    return callableLocation;
   }
 
   @Override
@@ -322,10 +282,6 @@ public String toString() {
       sb.append(", uri='");
       sb.append(uri);
     }
-    if (token != null) {
-      sb.append(", token='");
-      sb.append(token.getName());
-    }
     sb.append("'}");
 
     return sb.toString();

src/main/java/net/rptools/maptool/client/macro/MacroLocationFactory.java

@@ -31,7 +31,7 @@ public class MacroLocationFactory {
   private MacroLocationFactory() {}
 
   /** The singleton instance of the factory. */
-  private static MacroLocationFactory instance = new MacroLocationFactory();
+  private static final MacroLocationFactory instance = new MacroLocationFactory();
 
   /**
    * Returns the singleton instance of the factory.
@@ -49,7 +49,7 @@ public static MacroLocationFactory getInstance() {
    * @return a new {@link MacroLocation} object for an unknown location.
    */
   public MacroLocation createUnknownLocation(@Nonnull String name) {
-    return new MacroLocation(name, MacroSource.unknown, "");
+    return new MacroLocation(name, MacroSource.unknown, "", "");
   }
 
   /**
@@ -59,7 +59,11 @@ public MacroLocation createUnknownLocation(@Nonnull String name) {
    * @return a new {@link MacroLocation} object for a global Panel.
    */
   public MacroLocation createGlobalLocation(@Nonnull String name) {
-    return new MacroLocation(name, MacroSource.global, MacroSource.global.getSourceName());
+    return new MacroLocation(
+        name,
+        MacroSource.global,
+        MacroSource.global.getSourceName(),
+        MacroSource.global.getSourceName());
   }
 
   /**
@@ -69,7 +73,11 @@ public MacroLocation createGlobalLocation(@Nonnull String name) {
    * @return a new {@link MacroLocation} object for a campaign Panel.
    */
   public MacroLocation createCampaignLocation(@Nonnull String name) {
-    return new MacroLocation(name, MacroSource.campaign, MacroSource.campaign.getSourceName());
+    return new MacroLocation(
+        name,
+        MacroSource.campaign,
+        MacroSource.campaign.getSourceName(),
+        MacroSource.campaign.getSourceName());
   }
 
   /**
@@ -80,7 +88,7 @@ public MacroLocation createCampaignLocation(@Nonnull String name) {
    * @return a new {@link MacroLocation} object for a token.
    */
   public MacroLocation createTokenLocation(@Nonnull String name, @Nonnull Token token) {
-    return new MacroLocation(name, MacroSource.token, token.getName(), null, token);
+    return new MacroLocation(name, MacroSource.token, token.getName(), "Token:" + token.getName());
   }
 
   /**
@@ -92,7 +100,7 @@ public MacroLocation createTokenLocation(@Nonnull String name, @Nonnull Token to
    */
   public MacroLocation createLibTokenLocation(@Nonnull String name, @Nonnull Token libToken) {
     return new MacroLocation(
-        name, MacroSource.library, libToken.getName().substring(4), null, libToken);
+        name, MacroSource.library, libToken.getName().substring(4), libToken.getName());
   }
 
   /**
@@ -102,18 +110,22 @@ public MacroLocation createLibTokenLocation(@Nonnull String name, @Nonnull Token
    * @return a new {@link MacroLocation} object for a GM Panel.
    */
   public MacroLocation createGmLocation(@Nonnull String name) {
-    return new MacroLocation(name, MacroSource.gm, MacroSource.gm.getSourceName());
+    return new MacroLocation(
+        name, MacroSource.gm, MacroSource.gm.getSourceName(), MacroSource.gm.getSourceName());
   }
 
   /**
    * Creates a new {@link MacroLocation} object for a library location.
    *
-   * @param name the name of the macro.
+   * @param functionName the name of the function.
    * @return a new {@link MacroLocation} object for a library location.
    */
   public MacroLocation createExecFunctionLocation(@Nonnull String functionName) {
     return new MacroLocation(
-        MacroSource.execFunction.getSourceName(), MacroSource.execFunction, functionName);
+        MacroSource.execFunction.getSourceName(),
+        MacroSource.execFunction,
+        functionName,
+        functionName);
   }
 
   /**
@@ -126,6 +138,7 @@ public MacroLocation createMacroLinkLocation(@Nonnull String name) {
     return new MacroLocation(
         MacroSource.macroLink.getSourceName(),
         MacroSource.macroLink,
+        MacroSource.macroLink.getSourceName(),
         MacroSource.macroLink.getSourceName());
   }
 
@@ -136,13 +149,14 @@ public MacroLocation createMacroLinkLocation(@Nonnull String name) {
    * @return a new {@link MacroLocation} object for an event.
    */
   public MacroLocation createEventLocation(@Nonnull String name) {
-    return new MacroLocation(MacroSource.event.getSourceName(), MacroSource.event, name);
+    return new MacroLocation(MacroSource.event.getSourceName(), MacroSource.event, name, name);
   }
 
   public MacroLocation createSentryIoLoggingLocation() {
     return new MacroLocation(
         MacroSource.sentryIoLogging.getSourceName(),
         MacroSource.sentryIoLogging,
+        MacroSource.sentryIoLogging.getSourceName(),
         MacroSource.sentryIoLogging.getSourceName());
   }
 
@@ -154,29 +168,40 @@ public MacroLocation createSentryIoLoggingLocation() {
    * @return a new {@link MacroLocation} object for a URI location.
    */
   public MacroLocation createUriLocation(@Nonnull String name, @Nullable URI calledFrom) {
+    URI uri;
     try {
-      var uri = new URI(name);
-      if (uri.getScheme() == null) {
-        if (calledFrom == null) {
-          return createUnknownLocation(name);
-        }
-        uri = calledFrom.resolve(uri);
-      }
-      return new MacroLocation(uri.getPath(), MacroSource.uri, uri.getHost(), uri, null);
+      uri = new URI(name);
     } catch (URISyntaxException e) {
       return createUnknownLocation(name);
     }
+
+    if (uri.getHost() == null) {
+      if (calledFrom == null) {
+        return createUnknownLocation(name);
+      }
+      uri = calledFrom.resolve(uri);
+    }
+
+    var scheme = uri.getScheme();
+    if (scheme == null || !scheme.equalsIgnoreCase("lib")) {
+      return createUnknownLocation(name);
+    }
+
+    return new MacroLocation(
+        uri.getPath().substring(1), MacroSource.uri, uri.getHost(), uri.getHost(), uri);
   }
 
   /**
    * Creates a new {@link MacroLocation} object for the chat box.
    *
-   * @param token the token associated with the chat box.
    * @return a new {@link MacroLocation} object for a the chat box.
    */
   public MacroLocation createChatLocation() {
     return new MacroLocation(
-        MacroSource.chat.getSourceName(), MacroSource.chat, MacroSource.chat.getSourceName());
+        MacroSource.chat.getSourceName(),
+        MacroSource.chat,
+        MacroSource.chat.getSourceName(),
+        MacroSource.chat.getSourceName());
   }
 
   /**
@@ -186,11 +211,8 @@ public MacroLocation createChatLocation() {
    * @return a new {@link MacroLocation} object for a tooltip.
    */
   public MacroLocation createToolTipLocation(@Nullable Token token) {
+    var location = token != null ? token.getName() : "";
     return new MacroLocation(
-        MacroSource.tooltip.getSourceName(),
-        MacroSource.tooltip,
-        token != null ? token.getName() : "",
-        null,
-        token);
+        MacroSource.tooltip.getSourceName(), MacroSource.tooltip, location, location);
   }
 }

src/test/java/net/rptools/maptool/client/macro/MacroLocationFactory.java

@@ -89,7 +89,7 @@ void testCreateGmLocation() {
   void testCreateUriLocation() throws URISyntaxException {
     MacroLocation location =
         factory.createUriLocation("lib://test/macro/m2", new URI("lib://lib-test.net/macros/m1"));
-    assertEquals("/macro/m2", location.getName());
+    assertEquals("macro/m2", location.getName());
     assertEquals(MacroLocation.MacroSource.uri, location.getSource());
     assertEquals("test", location.getLocation());
     assertNotNull(location.getUri());
@@ -110,7 +110,7 @@ void testCreateUriLocationInvalid() throws URISyntaxException {
   void testCreateRelativeUriLocationRelative() throws URISyntaxException {
     MacroLocation location =
         factory.createUriLocation("relative/path", new URI("lib://test-lib.net/macros/m1"));
-    assertEquals("/macros/relative/path", location.getName());
+    assertEquals("macros/relative/path", location.getName());
     assertEquals(MacroLocation.MacroSource.uri, location.getSource());
     assertEquals("test-lib.net", location.getLocation());
     assertNotNull(location.getUri());