Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactored code to resolve implementation as well as design smells and increase maintainability. #233

Closed
wants to merge 4 commits into from
Closed

Refactored code to resolve implementation as well as design smells and increase maintainability. #233

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e506ab1
Assignment 1 Submission
Mar 2, 2024
441b836
Refactored code to resolve implementation code smells
Mar 29, 2024
abcd281
Updated Pull request with suggested changes
Mar 31, 2024
7db4d5e
Refactored code to increase maintainability
Mar 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions pom.xml
View file
Open in desktop
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see HintTextFieldTest.java

Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@
<version>5.12.1</version>
</dependency>

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>3.12.4</version> <!-- or the version you prefer -->
<scope>test</scope>
</dependency>

<!-- https://github.com/Osiris-Team/AutoPlug-Core -->
<dependency>
<groupId>com.github.Osiris-Team</groupId>
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/com/osiris/autoplug/client/configs/UpdaterConfig.java
View file
Open in desktop
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These names are long yes, but because they follow a standard like all the other fields in this class, which is to represent the yaml keys. Please revert these.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert this.
only functions strictly related to the config should be allowed here.

Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,14 @@ public class UpdaterConfig extends MyYaml {
public YamlSection plugins_updater_version;
public YamlSection plugins_updater_async;
public YamlSection plugins_updater_web_database;
public YamlSection plugins_updater_web_database_min_usages;
public YamlSection plugin_updater_min_usages;

public YamlSection mods_updater;
public YamlSection mods_updater_profile;
public YamlSection mods_updater_path;
public YamlSection mods_updater_version;
public YamlSection mods_updater_async;
public YamlSection mods_update_check_name_for_mod_loader;
public YamlSection mods_loader_update_checkName;


public UpdaterConfig() throws IOException, DuplicateKeyException, YamlReaderException, IllegalListException, NotLoadedException, IllegalKeyException, YamlWriterException {
Expand Down Expand Up @@ -206,7 +206,7 @@ public UpdaterConfig() throws IOException, DuplicateKeyException, YamlReaderExce
"Uses the AutoPlug-Web database to fill in missing plugin information.",
"This option is only available for premium servers.",
"Note that a server-key must be provided for this to work.");
plugins_updater_web_database_min_usages = put(name, "plugins-updater", "web-database", "min-usages").setDefValues("50").setComments(
plugin_updater_min_usages = put(name, "plugins-updater", "web-database", "min-usages").setDefValues("50").setComments(
"The minimum amount of usages in servers a piece of information has to have to be used and deemed reliable.");

put(name, "mods-updater").setCountTopLineBreaks(1);
Expand All @@ -221,7 +221,7 @@ public UpdaterConfig() throws IOException, DuplicateKeyException, YamlReaderExce
"Asynchronously checks for updates.",
"Normally this should be faster than checking for updates synchronously, thus it should be enabled.",
"The only downside of this is that your log file gets a bit messy.");
mods_update_check_name_for_mod_loader = put(name, "mods-updater", "check-name-for-mod-loader").setDefValues("false").setComments(
mods_loader_update_checkName = put(name, "mods-updater", "check-name-for-mod-loader").setDefValues("false").setComments(
"Only relevant for determining if a curseforge mod release is forge or fabric.",
"If enabled additionally checks the mod name to see if it contains fabric or forge.");

Expand Down
136 changes: 84 additions & 52 deletions src/main/java/com/osiris/autoplug/client/console/Commands.java
View file
Open in desktop
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are all those extra imports are needed?

Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,27 @@

package com.osiris.autoplug.client.console;

import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
import java.io.InputStreamReader;
import java.net.InetAddress;
import java.net.URL;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Properties;
import java.util.Scanner;

import org.jetbrains.annotations.NotNull;

import com.osiris.autoplug.client.Main;
import com.osiris.autoplug.client.Server;
import com.osiris.autoplug.client.configs.UpdaterConfig;
Expand All @@ -33,18 +54,14 @@
import com.osiris.autoplug.client.utils.UtilsMinecraft;
import com.osiris.autoplug.client.utils.tasks.MyBThreadManager;
import com.osiris.autoplug.client.utils.tasks.UtilsTasks;
import com.osiris.betterthread.exceptions.JLineLinkException;
import com.osiris.dyml.exceptions.DuplicateKeyException;
import com.osiris.dyml.exceptions.IllegalKeyException;
import com.osiris.dyml.exceptions.IllegalListException;
import com.osiris.dyml.exceptions.NotLoadedException;
import com.osiris.dyml.exceptions.YamlReaderException;
import com.osiris.dyml.exceptions.YamlWriterException;
import com.osiris.jlib.logger.AL;
import org.jetbrains.annotations.NotNull;

import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
import java.io.InputStreamReader;
import java.net.InetAddress;
import java.net.URL;
import java.nio.file.*;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.*;

/**
* Listens for input started with .
Expand Down Expand Up @@ -359,7 +376,7 @@ public static boolean installPlugin(String command) throws Exception {
if(input.startsWith(repo)){
int bukkitId = Integer.parseInt(input.replace(repo, "").trim());
result = new ResourceFinder().findPluginByBukkitId(new MinecraftPlugin(new File(pluginsDir + "/" + tempName).getAbsolutePath(),
tempName, "0", "", 0, bukkitId, ""));
tempName, "0", "", 0, bukkitId, ""));
}
repo = "github";
if(input.startsWith(repo)){
Expand All @@ -381,7 +398,7 @@ public static boolean installPlugin(String command) throws Exception {
result = new ResourceFinder().findPluginByModrinthId(plugin2, mcVersion);
}
/*
Nothing of the above worked thus do search by name
Nothing of the above worked thus do search by name
*/
if(!SearchResult.isMatchFound(result)){
plugin.setName(input.trim());
Expand Down Expand Up @@ -444,61 +461,78 @@ public static boolean installPlugin(String command) throws Exception {

public static boolean installMod(String command) throws Exception {
String input = command.replaceFirst("\\.install mod", "").replaceFirst("\\.im", "").trim();

SearchResult result = null;
String tempName = "NEW_MOD";
UpdaterConfig updaterConfig = new UpdaterConfig();
File modsDir = FileManager.convertRelativeToAbsolutePath(updaterConfig.mods_updater_path.asString());

String mcVersion = updaterConfig.mods_updater_version.asString();
if (mcVersion == null) mcVersion = Server.getMCVersion();
if (mcVersion == null)
mcVersion = Server.getMCVersion();
MinecraftMod mod = initializeMinecraftMod(modsDir, tempName, input);
if (mod != null) {
result = findMod(input, mod, mcVersion);
}

MinecraftMod mod = new MinecraftMod(new File(modsDir + "/" + tempName).getAbsolutePath(), tempName, "0",
"", "0", "0", "");
if (!SearchResult.isMatchFound(result)) {
AL.warn("Failed to find mod, check the provided name for typos.");
return false;
}
File finalDest = downloadMod(modsDir, result, tempName);
AL.info("Installed to: " + finalDest);
return true;
}

String repo = "modrinth";
if(input.startsWith(repo)) {
private static MinecraftMod initializeMinecraftMod(File modsDir, String tempName, String input) {
MinecraftMod mod = new MinecraftMod(new File(modsDir + "/" + tempName).getAbsolutePath(), tempName, "0", "",
"0", "0", "");
String repo;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must be initialized: String repo = "modrinth";

if (input.startsWith(repo = "modrinth")) {
mod.modrinthId = input.replace(repo, "").trim();
result = new ResourceFinder().findModByModrinthId(new InstalledModLoader(),
mod, mcVersion);
}
repo = "curseforge";
if(input.startsWith(repo)) {
} else if (input.startsWith(repo = "curseforge")) {
mod.curseforgeId = input.replace(repo, "").trim();
result = new ResourceFinder().findModByCurseforgeId(new InstalledModLoader(), mod,
mcVersion, updaterConfig.mods_update_check_name_for_mod_loader.asBoolean());
}
repo = "github";
if(input.startsWith(repo)) {
} else if (input.startsWith(repo = "github")) {
String[] split = input.replace(repo, "").trim().split(" ");
if (!input.contains("/")) {
AL.warn("The github format must be <author>/<name> (and optional <asset-name>).");
return false;
return null;
}
mod.githubRepoName = (split[0]);
if(split.length >= 2) mod.githubAssetName = (split[1]);
else mod.githubAssetName = (split[0].split("/")[1]);
result = new ResourceFinder().findByGithubUrl(mod);
}
/*
Nothing of the above worked thus do search by name
*/
if(!SearchResult.isMatchFound(result)){
if (split.length >= 2)
mod.githubAssetName = (split[1]);
else
mod.githubAssetName = (split[0].split("/")[1]);
} else {
mod.setName(input.trim());
}
return mod;
}

try { // SPIGOT
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep these comments

// TODO something similar for mods: result = new ResourceFinder().findUnknownSpigotPlugin(plugin);
} catch (Exception e) {
//AL.warn("Failed to find plugin named '"+plugin.getName()+"' at spigotmc.org.", e);
}
// TODO also search other repos
private static SearchResult findMod(String input, MinecraftMod mod, String mcVersion)
throws NotLoadedException, YamlReaderException, YamlWriterException, IOException, IllegalKeyException,
DuplicateKeyException, IllegalListException {
SearchResult result = null;
if (mod.modrinthId != null) {
result = new ResourceFinder().findModByModrinthId(new InstalledModLoader(), mod, mcVersion);
} else if (mod.curseforgeId != null) {
UpdaterConfig updaterConfig = new UpdaterConfig();
result = new ResourceFinder().findModByCurseforgeId(new InstalledModLoader(), mod, mcVersion,
updaterConfig.mods_loader_update_checkName.asBoolean());
} else if (mod.githubRepoName != null) {
result = new ResourceFinder().findByGithubUrl(mod);
}
return result;
}

if (!SearchResult.isMatchFound(result)) {
AL.warn("Failed to find mod, check the provided name for typos.");
return false;
private static File downloadMod(File modsDir, SearchResult result, String tempName)
throws NotLoadedException, YamlReaderException, YamlWriterException, IOException, IllegalKeyException,
DuplicateKeyException, IllegalListException {
UpdaterConfig updaterConfig = new UpdaterConfig();
MyBThreadManager myManager = null;
try {
myManager = new UtilsTasks().createManagerAndPrinter();
} catch (JLineLinkException e) {
throw new RuntimeException(e);
}
MyBThreadManager myManager = new UtilsTasks().createManagerAndPrinter();
File finalDest = new File(modsDir + "/" + result.mod.getName() + "-LATEST-[" + result.latestVersion + "].jar");
TaskModDownload task = new TaskModDownload("ModDownloader", myManager.manager, tempName, result.latestVersion,
result.downloadUrl, result.mod.ignoreContentType, "AUTOMATIC", finalDest);
Expand All @@ -512,9 +546,7 @@ public static boolean installMod(String command) throws Exception {
new File(mod2.installationPath).getName().replace(tempName, mod2.getName()));
}
}
AL.info("Installed to: " + finalDest);
return true;
return finalDest;
}


}
26 changes: 17 additions & 9 deletions src/main/java/com/osiris/autoplug/client/managers/FileManager.java
View file
Open in desktop
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the FileManager is basically a FileSearcher, no need to split that up, or if you really want to split it up, make each method into its own class, because yes they are pretty long.

Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@

package com.osiris.autoplug.client.managers;

import com.osiris.autoplug.client.utils.GD;
import com.osiris.jlib.logger.AL;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import static com.osiris.jprocesses2.util.OS.*;

import java.io.File;
import java.io.FileInputStream;
Expand All @@ -24,7 +21,11 @@
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;

import static com.osiris.jprocesses2.util.OS.isWindows;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import com.osiris.autoplug.client.utils.GD;
import com.osiris.jlib.logger.AL;

/**
* Search & find files!
Expand Down Expand Up @@ -369,21 +370,28 @@ public FileVisitResult visitFile(Path path,
@NotNull
@Override
public FileVisitResult preVisitDirectory(@NotNull Path dir, @NotNull BasicFileAttributes attrs) throws IOException {

//Skip subdirectories of non main worki dirs and non matching dirs
if (!dir.toString().equals(GD.WORKING_DIR.toString()) && attrs.isDirectory() && !pathMatcher.matches(dir.getFileName())) {
if (shouldSkipDirectory(dir, attrs)) {
return FileVisitResult.SKIP_SUBTREE;
} else if (!dir.toString().equals(GD.WORKING_DIR.toString())) {
//Adds folders to list
// Adds folders to list
queryFiles.add(new File(dir.toString()));
return FileVisitResult.CONTINUE;
} else {
return FileVisitResult.CONTINUE;
}
}

// Method to determine whether to skip the directory
private boolean shouldSkipDirectory(Path dir, BasicFileAttributes attrs) {
return !dir.equals(GD.WORKING_DIR) && attrs.isDirectory() && !matchesPattern(dir);
}

// Method to check if directory name matches a pattern
private boolean matchesPattern(Path dir) {
return pathMatcher.matches(dir.getFileName());
}


@NotNull
@Override
public FileVisitResult visitFileFailed(Path file, IOException exc)
Expand Down
Loading