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.

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

Auto add public trackers for magnet/torrent task #162

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions _locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,14 @@
}
}
},
"Automatically_add_trackers_to_new_tasks_form_a_trackslist_URL": {
"message": "Automatically add trackers to new tasks form a trackslist URL",
"description": "Label for option on settings page."
},
"URL_is_empty": {
"message": "URL is empty",
"description": "Error message when public trackerslist url is empty."
},
"Debugging_Output": {
"message": "Debugging Output",
"description": "Header for settings section."
Expand Down
12 changes: 10 additions & 2 deletions _locales/zh_CN/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,14 @@
}
}
},
"Automatically_add_trackers_to_new_tasks_form_a_trackslist_URL": {
"message": "添加BT磁力下载时自动添加以下订阅列表中的Trackers",
"description": "Label for option on settings page."
},
"URL_is_empty": {
"message": "URL 为空",
"description": "Error message when public trackerslist url is empty."
},
"Debugging_Output": {
"message": "调试输出",
"description": "Header for settings section."
Expand Down Expand Up @@ -475,15 +483,15 @@
"description": "Reassuring explanatory text for when we have a fatal rendering error."
},
"Please_": {
"message": "请 ",
"message": "请",
"description": "Prefix for action text for the popup when asking the user to file a bug when we have a fatal rendering error."
},
"file_a_bug": {
"message": "提交一个bug",
"description": "Live link text for action text for the popup when asking the user to file a bug when we have a fatal rendering error."
},
"_and_include_the_information_below": {
"message": " 并附带以下信息",
"message": "并附带以下信息",
"description": "Suffix for action text for the popup when asking the user to file a bug when we have a fatal rendering error."
},
"Clear_ZcountZ_Completed_Tasks": {
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
},
"dependencies": {
"axios": "^0.19.0",
"bencodec": "^2.4.0",
"chrome-extension-async": "^3.3.2",
"classnames": "^2.2.6",
"lodash-es": "^4.17.14",
Expand Down
1 change: 1 addition & 0 deletions src/background/actions/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from "./addDownloadTasksAndPoll";
export * from "./clearCachedTasks";
export * from "./torrentTracker";
export * from "./pollTasks";
70 changes: 70 additions & 0 deletions src/background/actions/torrentTracker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import Axios from "axios";
import bencodec from "bencodec";
import { startsWithAnyProtocol, MAGNET_PROTOCOL } from "../../common/apis/protocols";
import type { State } from "../../common/state";

let cachedTrackers: string[] = [];
let lastPublicTrackerURL = "";

async function updateRemoteTrackers(url: string) {
let response;

try {
response = await Axios.get(url, { timeout: 10000 });
lastPublicTrackerURL = url;
} catch (e) {
console.log("Axios Error caught when updating public trackers:", e);
cachedTrackers = [];
}

const trackerText: string = response?.data?.toString();

if (trackerText !== "") {
if (trackerText.includes(",")) {
cachedTrackers = trackerText.split(",");
} else if (trackerText.includes("\n\n")) {
cachedTrackers = trackerText.split("\n\n");
} else {
cachedTrackers = trackerText.split("\n");
}
console.log("successfully updated public trackers:", cachedTrackers.length);
}
}

export function updateAndGetTorrentTrackers(storedState: State): string[] {
console.debug("updateAndGetTorrentTrackers was called", new Error().stack);
console.debug("cached trackers:", cachedTrackers.length);

const flag = storedState.settings.torrentTrackers.enablePublicTrackers;
Copy link
Owner

Choose a reason for hiding this comment

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

I think there's a bug around this flag.

  1. User enables public trackers.
  2. The extension fetches and holds onto the list.
  3. User disables public trackers.
  4. User adds a new torrent task.
  5. Public trackers are added anyway.

I think something should clear the list if the flag is unset.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will fix later

const url = storedState.settings.torrentTrackers.publicTrackerURL;

if (flag && url !== lastPublicTrackerURL) {
updateRemoteTrackers(url);
}

return cachedTrackers;
}

export function setTrackers(trackers: string[]) {
cachedTrackers = trackers;
}

export function addTrackersToURL(url: string): string {
if (startsWithAnyProtocol(url, MAGNET_PROTOCOL)) {
url += url.includes("?") ? "" : "?";
cachedTrackers.some((t, i) => {
if (i >= 50) return true; // make sure uri is not too large
url += "&tr=" + encodeURIComponent(t);
return false;
});
}
return url;
}

export function addTrackersToMetaData(metaData: Buffer) {
const torrent: any = bencodec.decode(metaData);
cachedTrackers.forEach((t) => {
torrent["announce-list"].push([Buffer.from(t, "utf8")]);
});
return bencodec.encode(torrent);
}
5 changes: 5 additions & 0 deletions src/background/actions/urls.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Axios from "axios";
import { addTrackersToURL, addTrackersToMetaData } from "./torrentTracker";
import { parse as parseQueryString } from "query-string";
import {
ALL_DOWNLOADABLE_PROTOCOLS,
Expand Down Expand Up @@ -179,6 +180,9 @@ export async function resolveUrl(url: string): Promise<ResolvedUrl> {
return createUnexpectedError(e, "error while trying to fetch metadata file");
}

if (metadataFileType.mediaType === METADATA_FILE_TYPES[0].mediaType)
boin marked this conversation as resolved.
Show resolved Hide resolved
response.data = addTrackersToMetaData(response.data);

return {
type: "metadata-file",
url,
Expand All @@ -192,6 +196,7 @@ export async function resolveUrl(url: string): Promise<ResolvedUrl> {
};
}
} else if (startsWithAnyProtocol(url, ALL_DOWNLOADABLE_PROTOCOLS)) {
if (startsWithAnyProtocol(url, MAGNET_PROTOCOL)) url = addTrackersToURL(url);
return {
type: "direct-download",
url,
Expand Down
2 changes: 2 additions & 0 deletions src/background/backgroundState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export interface BackgroundState {
notificationInterval: number | undefined;
didInitializeSettings: boolean;
showNonErrorNotifications: boolean;
torrentTrackers: string[] | undefined;
}

const state: BackgroundState = {
Expand All @@ -21,6 +22,7 @@ const state: BackgroundState = {
notificationInterval: undefined,
didInitializeSettings: false,
showNonErrorNotifications: true,
torrentTrackers: undefined,
};

export function getMutableStateSingleton() {
Expand Down
4 changes: 3 additions & 1 deletion src/background/onStateChange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { SessionName } from "synology-typescript-api";
import { getMutableStateSingleton } from "./backgroundState";
import { getHostUrl, State } from "../common/state";
import { notify } from "../common/notify";
import { pollTasks, clearCachedTasks } from "./actions";
import { pollTasks, clearCachedTasks, updateAndGetTorrentTrackers } from "./actions";
import { assertNever } from "../common/lang";
import { filterTasks } from "../common/filtering";

Expand Down Expand Up @@ -51,6 +51,8 @@ export function onStoredStateChange(storedState: State) {
backgroundState.showNonErrorNotifications =
storedState.settings.notifications.enableFeedbackNotifications;

backgroundState.torrentTrackers = updateAndGetTorrentTrackers(storedState);
Copy link
Owner

Choose a reason for hiding this comment

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

I would prefer a shift of responsibility around this call that looks kind of like the notifications block, above. Something like:

  • The background task state keeps a copy of the settings, rather than the file in actions/.
  • When the settings changes, it's this code (and not updateAndGetTorrentTrackers) that decides if anything needs to be done.
  • It passes the bare minimum of information to other methods that they need to do their jobs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So your are suggesting not the module itself but backgroundStates to hold cachedTrackersList? I'll try it.

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah. backgroundState is the place to store anything long-lived that doesn't belong as part of the configuration of the extension. Earlier versions of the code actually used the pattern you had here, but I changed it because it got too hard to manage.


if (storedState.taskFetchFailureReason) {
browser.browserAction.setIcon({
path: {
Expand Down
10 changes: 10 additions & 0 deletions src/common/state/1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,18 @@ export type TaskSortType =
| "completed-percent-asc"
| "completed-percent-desc";

export interface TorrentTrackerSettings {
boin marked this conversation as resolved.
Show resolved Hide resolved
enablePublicTrackers: boolean;
publicTrackerURL: string;
}

export interface Settings {
connection: ConnectionSettings;
visibleTasks: VisibleTaskSettings;
taskSortType: TaskSortType;
notifications: NotificationSettings;
shouldHandleDownloadLinks: boolean;
torrentTrackers: TorrentTrackerSettings;
}

export interface CachedTasks {
Expand Down Expand Up @@ -77,6 +83,10 @@ export function transition(_state: null | undefined): State {
pollingInterval: 60,
},
shouldHandleDownloadLinks: true,
torrentTrackers: {
enablePublicTrackers: false,
publicTrackerURL: "",
},
cachedTasksVersion: 1,
tasks: [],
taskFetchFailureReason: null,
Expand Down
6 changes: 6 additions & 0 deletions src/common/state/2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
ConnectionSettings as ConnectionSettings_1,
VisibleTaskSettings as VisibleTaskSettings_1,
TaskSortType as TaskSortType_1,
TorrentTrackerSettings as TTSettings_1,
} from "./1";

export { Protocol, VisibleTaskSettings, TaskSortType, ConnectionSettings } from "./1";
Expand All @@ -22,6 +23,7 @@ export interface Settings {
taskSortType: TaskSortType_1;
notifications: NotificationSettings;
shouldHandleDownloadLinks: boolean;
torrentTrackers: TTSettings_1;
}

export interface CachedTasks {
Expand Down Expand Up @@ -60,6 +62,10 @@ export function transition(state: State_1): State {
enableCompletionNotifications: state.notifications.enabled,
completionPollingInterval: state.notifications.pollingInterval,
},
torrentTrackers: {
enablePublicTrackers: state.torrentTrackers.enablePublicTrackers,
publicTrackerURL: state.torrentTrackers.publicTrackerURL,
},
lastSevereError: undefined,
stateVersion: 2,
};
Expand Down
1 change: 1 addition & 0 deletions src/common/state/5.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const SETTINGS_KEYS = typesafeUnionMembers<keyof Settings_4>({
taskSortType: true,
notifications: true,
shouldHandleDownloadLinks: true,
torrentTrackers: true,
});

export function transition(state: State_4): State {
Expand Down
1 change: 1 addition & 0 deletions src/common/state/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
} from "./latest";
import { updateStateToLatest } from "./update";
import type { BadgeDisplayType } from "./4";
export type { TorrentTrackerSettings } from "./1";
import { typesafeUnionMembers, typesafeMapValues } from "../lang";
export * from "./listen";
export * from "./latest";
Expand Down
1 change: 1 addition & 0 deletions src/common/state/listen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const SETTING_NAMES = typesafeUnionMembers<keyof Settings>({
notifications: true,
shouldHandleDownloadLinks: true,
badgeDisplayType: true,
torrentTrackers: true,
});

const ALL_STORED_STATE_NAMES = typesafeUnionMembers<keyof State>({
Expand Down
54 changes: 54 additions & 0 deletions src/settings/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
SETTING_NAMES,
BadgeDisplayType,
ConnectionSettings,
TorrentTrackerSettings,
} from "../common/state";
import { BUG_REPORT_URL } from "../common/constants";
import { ResetClientSession } from "../common/apis/messages";
Expand All @@ -39,6 +40,7 @@ interface Props {
interface State {
savesFailed: boolean;
rawPollingInterval: string;
publicTrackerURL: string;
}

const POLL_MIN_INTERVAL = 15;
Expand All @@ -52,6 +54,7 @@ function isValidPollingInterval(stringValue: string) {
class SettingsForm extends React.PureComponent<Props, State> {
state: State = {
savesFailed: false,
publicTrackerURL: this.props.extensionState.settings.torrentTrackers.publicTrackerURL || "",
rawPollingInterval:
this.props.extensionState.settings.notifications.completionPollingInterval.toString() ||
POLL_DEFAULT_INTERVAL.toString(),
Expand Down Expand Up @@ -164,6 +167,44 @@ class SettingsForm extends React.PureComponent<Props, State> {
DOWNLOAD_ONLY_PROTOCOLS.join(", "),
])}
/>

<SettingsListCheckbox
checked={this.props.extensionState.settings.torrentTrackers.enablePublicTrackers}
onChange={() => {
this.setTorrentTrackers(
"enablePublicTrackers",
!this.props.extensionState.settings.torrentTrackers.enablePublicTrackers,
);
}}
label={browser.i18n.getMessage(
"Automatically_add_trackers_to_new_tasks_form_a_trackslist_URL",
boin marked this conversation as resolved.
Show resolved Hide resolved
)}
/>

<li>
<span className="indent" />
<input
type="text"
{...disabledPropAndClassName(
!this.props.extensionState.settings.torrentTrackers.enablePublicTrackers,
)}
style={{ flex: 1 }}
value={this.state.publicTrackerURL}
onChange={(e) => {
Copy link
Owner

Choose a reason for hiding this comment

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

Something in this chain of behavior needs to be debounced or otherwise delayed; as currently written, I think this is going to trigger a new request to load the tracker list on every keystroke, even while the URL is only partially written.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how about "onblur" event?

Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure how reliably onblur fires, especially if you do something like close the settings page without clicking outside of the input box. I'm imagining one of two solutions here:

  1. Debounce requests to the provided URL (you can use lodash-es/debounce), which doesn't involve any changes to the UI.
  2. Add a button to explicitly "save settings and fetch list", sort of like that which exists for the host/port/username/password configuration section. Changes to the UI required, but no clever debouncing or the like.

Having written these two down, I think the second is preferable: it's clearer about what's happening to the user, it follows existing UI patterns and it doesn't require much cleverness.

const publicTrackerURL = e.currentTarget.value;
this.setState({ publicTrackerURL });
if (publicTrackerURL !== "") {
this.setTorrentTrackers("publicTrackerURL", publicTrackerURL);
}
}}
/>
{this.props.extensionState.settings.torrentTrackers.enablePublicTrackers &&
this.state.publicTrackerURL === "" ? (
<span className="intent-error wrong-polling-interval">
{browser.i18n.getMessage("URL_is_empty")}
</span>
) : undefined}
</li>
</SettingsList>

{this.maybeRenderDebuggingOutputAndSeparator()}
Expand Down Expand Up @@ -259,6 +300,19 @@ ${this.props.lastSevereError}`;
});
}

private setTorrentTrackers<K extends keyof TorrentTrackerSettings>(
key: K,
value: TorrentTrackerSettings[K],
) {
console.log(key, value);
boin marked this conversation as resolved.
Show resolved Hide resolved
this.saveSettings({
torrentTrackers: {
...this.props.extensionState.settings.torrentTrackers,
[key]: value,
},
});
}

private saveSettings = async (settings: Partial<Settings>) => {
const success = await this.props.saveSettings({
...typesafePick(this.props.extensionState.settings, ...SETTING_NAMES),
Expand Down
Loading