qBittorrent icon indicating copy to clipboard operation
qBittorrent copied to clipboard

Make properties and info endpoint more consistent

Open qbittorrentfan opened this issue 2 years ago • 3 comments

Suggestion

Currently, the torrents/info and torrents/properties endpoint return different data:

  • properties does not return torrent name and TorrentID
  • some similar fields have different names: for example, added_on (info) vs addition_date

I believe it would be better to have consistent results returned on both endpoints.

Use case

Consuming the API as it is requires juggling between the info and properties endpoint because they don't return the same data. In my application, i'm actually using the info endpoint, systematically iterating over the whole torrent list due to #18185, but also because the properties endpoint misses some fields (name and TorrentID).

Extra info/examples/attachments

Some fields added recently in properties (infohash_v1/v2) are not documented yet on the API wiki page. Having only one list of torrent fields to maintain would make it easier to keep the docs and code in sync (this section of docs could also be auto-generated from parsing code).

Here's a small patch that does the minimum required effort to do what i want:

diff --git a/src/webui/api/torrentscontroller.cpp b/src/webui/api/torrentscontroller.cpp
index e998cbc7a..fe1e7df27 100644
--- a/src/webui/api/torrentscontroller.cpp
+++ b/src/webui/api/torrentscontroller.cpp
@@ -387,6 +387,9 @@ void TorrentsController::infoAction()
 //   - "save_path": Torrent save path
 //   - "download_path": Torrent download path
 //   - "comment": Torrent comment
+//   - "infohash_v1": Torrent v1 infohash (or empty string for v2 torrents)
+//   - "infohash_v2": Torrent v2 infohash (or empty string for v1 torrents)
+//   - "hash": Torrent TorrentID (infohashv1 for v1 torrents, truncated infohashv2 for v2/hybrid torrents)
+//   - "name": Torrent name
 void TorrentsController::propertiesAction()
 {
     requireParams({u"hash"_qs});
@@ -400,6 +403,8 @@ void TorrentsController::propertiesAction()
 
     dataDict[KEY_TORRENT_INFOHASHV1] = torrent->infoHash().v1().toString();
     dataDict[KEY_TORRENT_INFOHASHV2] = torrent->infoHash().v2().toString();
+    dataDict[KEY_TORRENT_NAME] = torrent->name();
+    dataDict[KEY_TORRENT_ID] = torrent->id().toString();
     dataDict[KEY_PROP_TIME_ELAPSED] = torrent->activeTime();
     dataDict[KEY_PROP_SEEDING_TIME] = torrent->finishedTime();
     dataDict[KEY_PROP_ETA] = static_cast<double>(torrent->eta());

In the future, i believe for consistency it would be useful to use the torrent serialization code (serialize() in api/serialize/serialize_torrent.cpp), which is already used for info endpoint, also in the properties endpoint. For backwards compatibility, we may want to keep the current properties endpoint, and introduce a new endpoint called "get" or "info_single".

qbittorrentfan avatar Dec 09 '22 11:12 qbittorrentfan

Demonstrating the possibility of a get endpoint returning same data as info endpoint:

diff --git a/src/webui/api/torrentscontroller.cpp b/src/webui/api/torrentscontroller.cpp
index fe1e7df27..4efe50f1c 100644
--- a/src/webui/api/torrentscontroller.cpp
+++ b/src/webui/api/torrentscontroller.cpp
@@ -1423,3 +1423,15 @@ void TorrentsController::exportAction()
 
     setResult(result.value());
 }
+
+void TorrentsController::getAction()
+{
+    requireParams({u"hash"_qs});
+
+    const auto id = BitTorrent::TorrentID::fromString(params()[u"hash"_qs]);
+    BitTorrent::Torrent *const torrent = BitTorrent::Session::instance()->getTorrent(id);
+    if (!torrent)
+        throw APIError(APIErrorType::NotFound);
+
+    setResult(QJsonObject::fromVariantMap(serialize(*torrent)));
+}
diff --git a/src/webui/api/torrentscontroller.h b/src/webui/api/torrentscontroller.h
index 779959ca4..18867c169 100644
--- a/src/webui/api/torrentscontroller.h
+++ b/src/webui/api/torrentscontroller.h
@@ -88,4 +88,5 @@ private slots:
     void renameFileAction();
     void renameFolderAction();
     void exportAction();
+    void getAction();
 };

Here's a test case demonstrating this:

#! /usr/bin/env bash
# Check difference between info/properties endpoints

api="http://localhost:8080/api/v2"
tempDir="$(mktemp -d)"
hash="caf1e1c30e81cb361b9ee167c4aa64228a7fa4fa9f6105232b28ad099f3a302e"

quit() {
	rm -R "$tempDir"
	exit $1
}

OUTPUT="$(curl --silent -c "$tempDir"/cookies.jar --data 'username=admin&password=adminadmin' "$api"/auth/login)"
if [ ! $? -eq 0 ]; then
	echo "Login request failed"
	quit 1
fi
if ! echo "$OUTPUT" | grep "Ok" 2>&1 >/dev/null; then
	echo "$OUTPUT"
	echo "Failed to login"
	quit 1
fi
echo "Logged in."

INFO="$(curl --silent -b "$tempDir"/cookies.jar --data "hashes=$hash" "$api"/torrents/info)"
if [ ! $? -eq 0 ]; then
	echo "FAIL INFO: connection"
	quit 1
fi
INFO="$(echo "$INFO" | jq --sort-keys .)"

PROPERTIES="$(curl --silent -b "$tempDir"/cookies.jar --data "hash=$hash" "$api"/torrents/get)"
if [ ! $? -eq 0 ]; then
	echo "FAIL PROPERTIES: connection"
	quit 1
fi
PROPERTIES="$(echo "$PROPERTIES" | jq --sort-keys .)"

diff --ignore-all-space <(echo "$INFO") <(echo "$PROPERTIES")

quit 0

It produces the following output, showing that the only difference is that the info endpoint returns a list:

Logged in.
1d0
< [
53d51
< ]

qbittorrentfan avatar Dec 09 '22 11:12 qbittorrentfan

I'd like the torrent comment to be included in the info. A lot of trackers use this for a link or torrentid (numeric id specific to the tracker), which is useful for interacting with their API. As-is, I pull the torrent list from 'info' then iterate over it to get the 'comment' field.

I agree that 'name' should be in the properties. I can't really see why you'd want the hash in the properties since you need the hash to query the properties, and thus already have it, but I don't really see anything against it either.

As far as consistency, that's always nice...

Jorsher avatar Dec 10 '22 23:12 Jorsher

PRs are welcome.

glassez avatar Dec 12 '22 11:12 glassez

@qbittorrentfan is this ticket still relevant ?

luzpaz avatar Aug 01 '24 13:08 luzpaz