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

Fix: wrong YouTube playlist detection #630

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix: wrong YouTube playlist detection #630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
85e3f8e
Fix: Added code to Standardized YouTube link format for issue #601
Tarunkumarkanakam Sep 2, 2024
bfb3051
Add: Added extractQueryParams function for issue #601
Tarunkumarkanakam Sep 2, 2024
17a956c
Minor tweaks for issue 601
Tarunkumarkanakam Sep 9, 2024
609b852
Moved Youtube URL formatting to Utility file
Tarunkumarkanakam Sep 30, 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
16 changes: 16 additions & 0 deletions CLI/src/main/java/main/Drifty_CLI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import utils.Logger;

import java.io.*;
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.Charset;
Expand Down Expand Up @@ -294,6 +295,21 @@ public static void main(String[] args) {
if (isInstagram(link)) {
link = formatInstagramLink(link);
}
else if (isYoutube(link)) {
try {
link = formatYoutubeLink(link);
} catch (IllegalArgumentException | URISyntaxException | MalformedURLException e) {
messageBroker.msgLinkError("Failed to process the YouTube link: " + e.getMessage());
messageBroker.msgInputInfo(QUIT_OR_CONTINUE, true);
String choice = SC.next().toLowerCase().strip();
if ("q".equals(choice)) {
LOGGER.log(MessageType.INFO, CLI_APPLICATION_TERMINATED);
break;
}
printBanner();
continue;
}
}
messageBroker.msgFilenameInfo("Retrieving filename from link...");
fileName = findFilenameInLink(link);
if (fileName != null && !fileName.isEmpty()) {
Expand Down
95 changes: 91 additions & 4 deletions Core/src/main/java/utils/Utility.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@
import org.apache.commons.text.StringEscapeUtils;
import org.hildan.fxgson.FxGson;
import preferences.AppSettings;
import properties.MessageCategory;
import properties.Mode;
import properties.OS;
import properties.Program;
import properties.*;

import java.io.*;
import java.net.*;
Expand All @@ -33,6 +30,7 @@

import static properties.Program.YT_DLP;
import static support.Constants.*;
import static utils.Utility.extractQueryParams;

public class Utility {
private static final Random RANDOM_GENERATOR = new Random(System.currentTimeMillis());
Expand Down Expand Up @@ -860,4 +858,93 @@ public static int parseStringToInt(String string, String errorMessage, MessageCa
return 0;
}
}

public static String formatYoutubeLink(String link) throws MalformedURLException, URISyntaxException {
String videoId = null;
URI uri = URI.create(link);
String domain = uri.getHost();

if ("youtu.be".equals(domain)) {
String path = uri.getPath();
if (path != null && path.length() > 1) {
videoId = path.substring(1); // removing the leading "/"
}
}

else if ("www.youtube.com".equals(domain) || "youtube.com".equals(domain)) {
Map<String, String> queryParams = extractQueryParams(link, "v");
videoId = queryParams.get("v");
}

if (videoId != null) {
uri = new URI("https", "www.youtube.com", "/watch", "v=" + videoId, null);
link = uri.toString();
} else {
throw new MalformedURLException(link + " is not a valid youtube link!");
}

return link;
}

/**
* Extracts the specified query parameters from the given URL. If no parameter names are provided (null or empty), all parameters are returned.
*
* @param urlLink The URL string from which to extract parameters.
* @param paramNames The names of the query parameters to extract. If null or empty, all parameters will be returned.
* @return A map containing the query parameter names and their corresponding values.
*/
public static Map<String, String> extractQueryParams(String urlLink, String... paramNames) {
Map<String, String> paramMap = new HashMap<>();

URL url = null;
try {
url = URI.create(urlLink).toURL();
} catch (MalformedURLException e) {
msgBroker.msgLinkError("Connection to the link timed out! Please check your internet connection. " + e.getMessage());
}
String query = url != null ? url.getQuery() : null;

// query is null or empty, return an empty map (no query parameters)
if (query == null || query.isEmpty()) {
return paramMap;
}

// splitting query string into individual parameters
String[] params = query.split("&");

// check if specific parameters are requested or if all should be returned
boolean returnAllParams = (paramNames == null || paramNames.length == 0);

for (String param : params) {
String[] pair = param.split("=");
if (pair.length == 2) {
String paramName = pair[0];
String paramValue = pair[1];

// add parameter to the map if it's requested or if all parameters should be returned
if (returnAllParams || contains(paramNames, paramName)) {
paramMap.put(paramName, paramValue);
}
}
}

return paramMap;
}
Comment on lines +889 to +932
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve error handling and efficiency in extractQueryParams.

The method correctly extracts query parameters, but there are opportunities for improvement:

  1. Error handling can be enhanced by returning early if the URL is malformed.
  2. The method can be more efficient by avoiding unnecessary null checks after URL creation.
  3. Handling of parameters without values can be improved.

Consider applying these changes:

 public static Map<String, String> extractQueryParams(String urlLink, String... paramNames) {
     Map<String, String> paramMap = new HashMap<>();

-    URL url = null;
     try {
-        url = URI.create(urlLink).toURL();
+        URL url = URI.create(urlLink).toURL();
+        String query = url.getQuery();
+
+        if (query == null || query.isEmpty()) {
+            return paramMap;
+        }
+
+        String[] params = query.split("&");
+        boolean returnAllParams = (paramNames == null || paramNames.length == 0);
+
+        for (String param : params) {
+            String[] pair = param.split("=", 2);
+            String paramName = pair[0];
+            String paramValue = pair.length > 1 ? pair[1] : "";
+
+            if (returnAllParams || contains(paramNames, paramName)) {
+                paramMap.put(paramName, paramValue);
+            }
+        }
     } catch (MalformedURLException e) {
-        msgBroker.msgLinkError("Connection to the link timed out! Please check your internet connection. " + e.getMessage());
+        msgBroker.msgLinkError("Malformed URL: " + e.getMessage());
     }
-    String query = url != null ? url.getQuery() : null;
-
-    if (query == null || query.isEmpty()) {
-        return paramMap;
-    }
-
-    String[] params = query.split("&");
-    boolean returnAllParams = (paramNames == null || paramNames.length == 0);
-
-    for (String param : params) {
-        String[] pair = param.split("=");
-        if (pair.length == 2) {
-            String paramName = pair[0];
-            String paramValue = pair[1];
-
-            if (returnAllParams || contains(paramNames, paramName)) {
-                paramMap.put(paramName, paramValue);
-            }
-        }
-    }

     return paramMap;
 }

These changes address the issues mentioned in the past review comment and improve the overall efficiency and robustness of the method.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Extracts the specified query parameters from the given URL. If no parameter names are provided (null or empty), all parameters are returned.
*
* @param urlLink The URL string from which to extract parameters.
* @param paramNames The names of the query parameters to extract. If null or empty, all parameters will be returned.
* @return A map containing the query parameter names and their corresponding values.
*/
public static Map<String, String> extractQueryParams(String urlLink, String... paramNames) {
Map<String, String> paramMap = new HashMap<>();
URL url = null;
try {
url = URI.create(urlLink).toURL();
} catch (MalformedURLException e) {
msgBroker.msgLinkError("Connection to the link timed out! Please check your internet connection. " + e.getMessage());
}
String query = url != null ? url.getQuery() : null;
// query is null or empty, return an empty map (no query parameters)
if (query == null || query.isEmpty()) {
return paramMap;
}
// splitting query string into individual parameters
String[] params = query.split("&");
// check if specific parameters are requested or if all should be returned
boolean returnAllParams = (paramNames == null || paramNames.length == 0);
for (String param : params) {
String[] pair = param.split("=");
if (pair.length == 2) {
String paramName = pair[0];
String paramValue = pair[1];
// add parameter to the map if it's requested or if all parameters should be returned
if (returnAllParams || contains(paramNames, paramName)) {
paramMap.put(paramName, paramValue);
}
}
}
return paramMap;
}
/**
* Extracts the specified query parameters from the given URL. If no parameter names are provided (null or empty), all parameters are returned.
*
* @param urlLink The URL string from which to extract parameters.
* @param paramNames The names of the query parameters to extract. If null or empty, all parameters will be returned.
* @return A map containing the query parameter names and their corresponding values.
*/
public static Map<String, String> extractQueryParams(String urlLink, String... paramNames) {
Map<String, String> paramMap = new HashMap<>();
try {
URL url = URI.create(urlLink).toURL();
String query = url.getQuery();
if (query == null || query.isEmpty()) {
return paramMap;
}
String[] params = query.split("&");
boolean returnAllParams = (paramNames == null || paramNames.length == 0);
for (String param : params) {
String[] pair = param.split("=", 2);
String paramName = pair[0];
String paramValue = pair.length > 1 ? pair[1] : "";
if (returnAllParams || contains(paramNames, paramName)) {
paramMap.put(paramName, paramValue);
}
}
} catch (MalformedURLException e) {
msgBroker.msgLinkError("Malformed URL: " + e.getMessage());
}
return paramMap;
}


/**
* Helper method to check if an array contains a specific value.
*
* @param array The array to check.
* @param value The value to search for.
* @return True if the array contains the value, false otherwise.
*/
private static boolean contains(String[] array, String value) {
if (array == null) return false;
for (String item : array) {
if (item.equals(value)) {
return true;
}
}
return false;
}
Comment on lines +934 to +949
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using Java's built-in method for array contains check.

The contains method is correctly implemented, but we can improve its efficiency by using Java's built-in methods.

Consider replacing the method with this more concise and potentially more efficient version:

-private static boolean contains(String[] array, String value) {
-    if (array == null) return false;
-    for (String item : array) {
-        if (item.equals(value)) {
-            return true;
-        }
-    }
-    return false;
-}
+private static boolean contains(String[] array, String value) {
+    return array != null && Arrays.asList(array).contains(value);
+}

This change utilizes Java's Arrays.asList(array).contains(value) method, which may be more efficient, especially for larger arrays. It also maintains the null check for the input array.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Helper method to check if an array contains a specific value.
*
* @param array The array to check.
* @param value The value to search for.
* @return True if the array contains the value, false otherwise.
*/
private static boolean contains(String[] array, String value) {
if (array == null) return false;
for (String item : array) {
if (item.equals(value)) {
return true;
}
}
return false;
}
/**
* Helper method to check if an array contains a specific value.
*
* @param array The array to check.
* @param value The value to search for.
* @return True if the array contains the value, false otherwise.
*/
private static boolean contains(String[] array, String value) {
return array != null && Arrays.asList(array).contains(value);
}

}
Loading