Skip to content

Commit

Permalink
Simplify link localization code
Browse files Browse the repository at this point in the history
Main goal of this simplification is to remove 'map name' from resource loader.
One place that is used is in Link Localization. Instead of passing a resource
loader to the link localizer, we can instead pass a file path which in turn
simplifies how link localization can be done.

Additional:
- removed link localization cache
- changed link localizatoin behavior to only localize links that are not already
  absolute. Previously links in single quotes would be localized and links in
  double quotes would be left as-is. The localization of fully qualified links
  would parse the link path, get the file name and localize the file name path.
  This is removed as it is difficult to understand and is much easier if
  we localize any relative links but keep absolute links as they are (regardless
  of quoting).
  • Loading branch information
DanVanAtta committed Jan 17, 2021
1 parent 16fcfd3 commit 4da2283
Show file tree
Hide file tree
Showing 9 changed files with 99 additions and 125 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ public static Optional<File> findPathToMapFolder(final String mapName) {
return FileSystemMapFinder.getPath(mapName);
}

public static File findPathToMapFolderOrElseThrow(final String mapName) {
return FileSystemMapFinder.getPath(mapName)
.orElseThrow(() -> new IllegalStateException("Unable to find map: " + mapName));
}

public List<String> getSortedGameList() {
return availableGames.stream()
.map(DefaultGameChooserEntry::getGameName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,10 @@ private static String buildGameNotesText(final URI gameUri) {
.map(PropertyList.Property::getValue)
.ifPresent(
mapName ->
notes.append(LocalizeHtml.localizeImgLinksInHtml(gameNotes, mapName))));
notes.append(
LocalizeHtml.localizeImgLinksInHtml(
gameNotes,
DownloadedMaps.findPathToMapFolderOrElseThrow(mapName)))));
return notes.toString();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import games.strategy.engine.data.PlayerList;
import games.strategy.engine.data.Territory;
import games.strategy.engine.delegate.IDelegateBridge;
import games.strategy.engine.framework.map.file.system.loader.DownloadedMaps;
import games.strategy.engine.message.IRemote;
import games.strategy.triplea.Constants;
import games.strategy.triplea.Properties;
Expand All @@ -16,6 +17,7 @@
import games.strategy.triplea.attachments.TerritoryAttachment;
import games.strategy.triplea.attachments.TriggerAttachment;
import games.strategy.triplea.formatter.MyFormatter;
import games.strategy.triplea.ui.UiContext;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.Collection;
Expand Down Expand Up @@ -319,7 +321,9 @@ public void signalGameOver(
stopGame = true;
} else {
// now tell the HOST, and see if they want to continue the game.
String displayMessage = LocalizeHtml.localizeImgLinksInHtml(status);
String displayMessage =
LocalizeHtml.localizeImgLinksInHtml(
status, DownloadedMaps.findPathToMapFolderOrElseThrow(UiContext.getMapDir()));
if (displayMessage.endsWith("</body>")) {
displayMessage =
displayMessage.substring(0, displayMessage.length() - "</body>".length())
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package games.strategy.triplea.ui;

import games.strategy.engine.framework.map.file.system.loader.DownloadedMaps;
import java.awt.Color;
import javax.swing.JEditorPane;
import javax.swing.JScrollPane;
Expand All @@ -10,7 +11,11 @@ public class NotesPanel extends JScrollPane {
private static final long serialVersionUID = 2746643868463714526L;

public NotesPanel(final String trimmedNotes) {
super(createNotesPane(LocalizeHtml.localizeImgLinksInHtml(trimmedNotes)));
super(
createNotesPane(
LocalizeHtml.localizeImgLinksInHtml(
trimmedNotes,
DownloadedMaps.findPathToMapFolderOrElseThrow(UiContext.getMapDir()))));
}

private static JEditorPane createNotesPane(final String html) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import games.strategy.engine.data.GamePlayer;
import games.strategy.engine.data.UnitType;
import games.strategy.engine.framework.map.file.system.loader.DownloadedMaps;
import games.strategy.triplea.ResourceLoader;
import games.strategy.triplea.attachments.UnitAttachment;
import java.io.IOException;
Expand Down Expand Up @@ -54,15 +55,18 @@ public static TooltipProperties getInstance() {
public String getTooltip(final UnitType unitType, final GamePlayer gamePlayer) {
final String customTip = getToolTip(unitType, gamePlayer, false);
if (!customTip.isEmpty()) {
return LocalizeHtml.localizeImgLinksInHtml(customTip);
return LocalizeHtml.localizeImgLinksInHtml(
customTip, DownloadedMaps.findPathToMapFolderOrElseThrow(UiContext.getMapDir()));
}
final String generated =
UnitAttachment.get(unitType)
.toStringShortAndOnlyImportantDifferences(
(gamePlayer == null ? GamePlayer.NULL_PLAYERID : gamePlayer));
final String appendedTip = getToolTip(unitType, gamePlayer, true);
if (!appendedTip.isEmpty()) {
return generated + LocalizeHtml.localizeImgLinksInHtml(appendedTip);
return generated
+ LocalizeHtml.localizeImgLinksInHtml(
appendedTip, DownloadedMaps.findPathToMapFolderOrElseThrow(UiContext.getMapDir()));
}
return generated;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import games.strategy.engine.framework.IGame;
import games.strategy.engine.framework.LocalPlayers;
import games.strategy.engine.framework.ServerGame;
import games.strategy.engine.framework.map.file.system.loader.DownloadedMaps;
import games.strategy.engine.framework.startup.ui.InGameLobbyWatcherWrapper;
import games.strategy.engine.framework.ui.SaveGameFileChooser;
import games.strategy.engine.history.HistoryNode;
Expand Down Expand Up @@ -919,7 +920,9 @@ public FightBattleDetails getBattle(

/** We do NOT want to block the next player from beginning their turn. */
public void notifyError(final String message) {
final String displayMessage = LocalizeHtml.localizeImgLinksInHtml(message);
final String displayMessage =
LocalizeHtml.localizeImgLinksInHtml(
message, DownloadedMaps.findPathToMapFolderOrElseThrow(UiContext.getMapDir()));
messageAndDialogThreadPool.submit(
() ->
EventThreadJOptionPane.showMessageDialogWithScrollPane(
Expand Down Expand Up @@ -954,7 +957,9 @@ public void notifyMessage(final String message, final String title) {
&& !getUiContext().getShowEndOfTurnReport()) {
return;
}
final String displayMessage = LocalizeHtml.localizeImgLinksInHtml(message);
final String displayMessage =
LocalizeHtml.localizeImgLinksInHtml(
message, DownloadedMaps.findPathToMapFolderOrElseThrow(UiContext.getMapDir()));
messageAndDialogThreadPool.submit(
() ->
EventThreadJOptionPane.showMessageDialogWithScrollPane(
Expand Down
66 changes: 19 additions & 47 deletions game-core/src/main/java/org/triplea/util/LocalizeHtml.java
Original file line number Diff line number Diff line change
@@ -1,58 +1,44 @@
package org.triplea.util;

import com.google.common.annotations.VisibleForTesting;
import games.strategy.triplea.ResourceLoader;
import games.strategy.triplea.ui.UiContext;
import java.net.URL;
import java.util.HashMap;
import java.util.Map;
import java.io.File;
import java.nio.file.Path;
import java.util.Optional;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import lombok.experimental.UtilityClass;
import lombok.extern.slf4j.Slf4j;
import org.triplea.io.FileUtils;

/**
* Provides methods that convert relative links within a game description into absolute links that
* will work on the local system.
* will work on the local system. Links that are already absolute (those beginning with 'http') will
* not be changed.
*/
@Slf4j
@UtilityClass
public final class LocalizeHtml {
private static final String ASSET_IMAGE_FOLDER = "doc/images/";
private static final String ASSET_IMAGE_NOT_FOUND = "notFound.png";
// Match the <img> src
private static final Pattern PATTERN_HTML_IMG_SRC_TAG =
Pattern.compile(
"(<img[^>]*src\\s*=\\s*)(?:\"([^\"]+)\"|'([^']+)')([^>]*/?>)", Pattern.CASE_INSENSITIVE);

/**
* This is only useful once we are IN a game. Before we go into the game, resource loader will
* either be null, or be the last game's resource loader.
* Replaces relative image links within a given {@code htmlText} with absolute links that point to
* the correct location on the local file system.
*/
public static String localizeImgLinksInHtml(final String htmlText) {
return localizeImgLinksInHtml(htmlText, UiContext.getResourceLoader());
}

/**
* Replaces relative image links within the HTML document {@code htmlText} with absolute links
* that point to the correct location on the local file system.
*/
public static String localizeImgLinksInHtml(final String htmlText, final String mapNameDir) {
return htmlText == null || mapNameDir == null || mapNameDir.isBlank()
? htmlText
: localizeImgLinksInHtml(htmlText, new ResourceLoader(mapNameDir));
}

@VisibleForTesting
static String localizeImgLinksInHtml(final String htmlText, final ResourceLoader loader) {
public static String localizeImgLinksInHtml(final String htmlText, final File mapContentFolder) {
if (htmlText == null) {
return null;
}
final StringBuilder result = new StringBuilder();
final Map<String, String> cache = new HashMap<>();
final Matcher matcher = PATTERN_HTML_IMG_SRC_TAG.matcher(htmlText);
while (matcher.find()) {
final String link = Optional.ofNullable(matcher.group(2)).orElseGet(() -> matcher.group(3));
assert link != null && !link.isEmpty() : "RegEx is broken";
final String localized = cache.computeIfAbsent(link, l -> getLocalizedLink(l, loader));

final boolean linkIsAbsolute = link.startsWith("http");
final String localized = linkIsAbsolute ? link : getLocalizedLink(link, mapContentFolder);

final char quote = matcher.group(2) != null ? '"' : '\'';
matcher.appendReplacement(
result, matcher.group(1) + quote + localized + quote + matcher.group(4));
Expand All @@ -62,23 +48,9 @@ static String localizeImgLinksInHtml(final String htmlText, final ResourceLoader
return result.toString();
}

private static String getLocalizedLink(final String link, final ResourceLoader loader) {
// remove full parent path
final String imageFileName = link.substring(Math.max(link.lastIndexOf("/") + 1, 0));

// replace when testing with: "REPLACEMENTPATH/" + imageFileName;
final String firstOption = ASSET_IMAGE_FOLDER + imageFileName;
URL replacementUrl = loader.getResource(firstOption);

if (replacementUrl == null) {
log.error(String.format("Could not find: %s/%s", loader.getMapName(), firstOption));
final String secondFallback = ASSET_IMAGE_FOLDER + ASSET_IMAGE_NOT_FOUND;
replacementUrl = loader.getResource(secondFallback);
if (replacementUrl == null) {
log.error(String.format("Could not find: %s", secondFallback));
return link;
}
}
return replacementUrl.toString();
private static String getLocalizedLink(final String link, final File mapContentFolder) {
final Path imagesPath =
mapContentFolder.toPath().resolve("doc").resolve("images").resolve(link);
return FileUtils.toUrl(imagesPath).toString();
}
}
103 changes: 32 additions & 71 deletions game-core/src/test/java/org/triplea/util/LocalizeHtmlTest.java
Original file line number Diff line number Diff line change
@@ -1,85 +1,46 @@
package org.triplea.util;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import games.strategy.triplea.ResourceLoader;
import java.net.URL;
import java.io.File;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

class LocalizeHtmlTest {
private final ResourceLoader loader = mock(ResourceLoader.class);
private final String testHtml =
"<audio src='test-audio'> &lt;img src=&quot;test&quot;&gt;"
+ "<img useless fill src=\"dir/actual-link\" alt='Alternative Text' > "
+ "<p> Placeholder </P> <img\n"
+ " src='another-link.png' />";

@Test
void testLocalizeHtml() throws Exception {

when(loader.getResource("doc/images/actual-link")).thenReturn(new URL("http://local-link-1"));
when(loader.getResource("doc/images/another-link.png"))
.thenReturn(new URL("http://local-link-2"));

final String result = LocalizeHtml.localizeImgLinksInHtml(testHtml, loader);

assertThat(result, not(containsString("dir/actual-link")));
assertThat(result, not(containsString("another-link.png")));

assertThat(result, containsString("http://local-link-1"));
assertThat(result, containsString("http://local-link-2"));

assertThat(result, containsString("test-audio"));

verify(loader, times(2)).getResource(any());
}

@Test
void testFallbackLink() throws Exception {
when(loader.getResource("doc/images/actual-link")).thenReturn(null);
when(loader.getResource("doc/images/another-link.png")).thenReturn(null);
when(loader.getResource("doc/images/notFound.png"))
.thenReturn(null, new URL("http://notFound.png"));
final String result = LocalizeHtml.localizeImgLinksInHtml(testHtml, loader);
assertThat(result, containsString("dir/actual-link"));
assertThat(result, containsString("http://notFound.png"));
assertThat(result, not(containsString("another-link.png")));
void testLocalizeHtml() {
final String testHtml =
"<audio src='test-audio'> &lt;img src=&quot;test&quot;&gt;"
+ "<img useless fill src=\"dir/actual-link\" alt='Alternative Text' > "
+ "<p> Placeholder </P> <img\n"
+ " src='another-link.png'/><img src=\"another-link\"/>";

final String result = LocalizeHtml.localizeImgLinksInHtml(testHtml, new File("/usr/local"));

assertThat(
result,
is(
"<audio src='test-audio'> &lt;img src=&quot;test&quot;&gt;"
+ "<img useless fill src=\"file:/usr/local/doc/images/dir/actual-link\" "
+ "alt='Alternative Text' > <p> Placeholder </P> <img\n"
+ " src='file:/usr/local/doc/images/another-link.png'/>"
+ "<img src=\"file:/usr/local/doc/images/another-link\"/>"));
}

@Test
void testDoNothingWithoutResources() {
assertThat(LocalizeHtml.localizeImgLinksInHtml(testHtml, loader), is(equalTo(testHtml)));
}

@Test
void testDoNothingWithoutTag() {
final String testHtml = "<p>Paragraph</p>";
assertThat(LocalizeHtml.localizeImgLinksInHtml(testHtml, loader), is(equalTo(testHtml)));
verify(loader, never()).getResource(any());
}

@Test
void testCaching() throws Exception {
final String testHtml = "<img src='test'><img src='test'>";
when(loader.getResource("doc/images/test")).thenReturn(new URL("http://test"));
LocalizeHtml.localizeImgLinksInHtml(testHtml, loader);
verify(loader, times(1)).getResource(any());
}

@Test
void testStrictHtml() {
final String testHtml = "<img src=\"test\"";
assertThat(LocalizeHtml.localizeImgLinksInHtml(testHtml, loader), is(equalTo(testHtml)));
verify(loader, never()).getResource(any());
@ParameterizedTest
@ValueSource(
strings = {
"<img src=http://someurl/>",
"<img src=\"http://someurl\"/>",
"<img\nsrc=\"http://someurl\"/>",
"<p>Paragraph</p>"
})
void testAbsoluteUrl(final String testHtml) {
assertThat(
LocalizeHtml.localizeImgLinksInHtml(testHtml, new File("/usr/local")),
is(equalTo(testHtml)));
}
}
15 changes: 15 additions & 0 deletions java-extras/src/main/java/org/triplea/io/FileUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import static com.google.common.base.Preconditions.checkNotNull;

import java.io.File;
import java.net.MalformedURLException;
import java.net.URL;
import java.nio.file.Path;
import java.util.Collection;
import java.util.List;
Expand Down Expand Up @@ -34,4 +36,17 @@ public static Collection<File> listFiles(final File directory) {
checkNotNull(directory);
return Optional.ofNullable(directory.listFiles()).map(List::of).orElseGet(List::of);
}

public static URL toUrl(final File file) {
return toUrl(file.toPath());
}

public static URL toUrl(final Path file) {
try {
return file.toUri().toURL();
} catch (final MalformedURLException e) {
throw new IllegalStateException(
"Invalid conversion from file to URL, file: " + file.toFile().getAbsolutePath(), e);
}
}
}

0 comments on commit 4da2283

Please sign in to comment.