Update /<count>/ endpoints to use a '?count=' query parameter instead

In most RESTful APIs, path parameters are used to represent resources, and
query parameters are used to control how these resources are being filtered/sorted/...

The old /<count>/ functionality is kept alive to maintain backwards compatibility,
but new paths with query parameters are introduced and documented as the default
interface so future API methods don't break consistency by using query parameters.

1 files changed, 30 insertions, 15 deletions

diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py
index a3d949c6a8..4f8676ec53 100755
--- a/test/functional/interface_rest.py
+++ b/test/functional/interface_rest.py
@@ -10,6 +10,7 @@ import http.client
 from io import BytesIO
 import json
 from struct import pack, unpack
+import typing
 import urllib.parse
 
 
@@ -57,14 +58,21 @@ class RESTTest (BitcoinTestFramework):
             args.append("-whitelist=noban@127.0.0.1")
         self.supports_cli = False
 
-    def test_rest_request(self, uri, http_method='GET', req_type=ReqType.JSON, body='', status=200, ret_type=RetType.JSON):
+    def test_rest_request(
+            self,
+            uri: str,
+            http_method: str = 'GET',
+            req_type: ReqType = ReqType.JSON,
+            body: str = '',
+            status: int = 200,
+            ret_type: RetType = RetType.JSON,
+            query_params: typing.Dict[str, typing.Any] = None,
+            ) -> typing.Union[http.client.HTTPResponse, bytes, str, None]:
         rest_uri = '/rest' + uri
-        if req_type == ReqType.JSON:
-            rest_uri += '.json'
-        elif req_type == ReqType.BIN:
-            rest_uri += '.bin'
-        elif req_type == ReqType.HEX:
-            rest_uri += '.hex'
+        if req_type in ReqType:
+            rest_uri += f'.{req_type.name.lower()}'
+        if query_params:
+            rest_uri += f'?{urllib.parse.urlencode(query_params)}'
 
         conn = http.client.HTTPConnection(self.url.hostname, self.url.port)
         self.log.debug(f'{http_method} {rest_uri} {body}')
@@ -83,6 +91,8 @@ class RESTTest (BitcoinTestFramework):
         elif ret_type == RetType.JSON:
             return json.loads(resp.read().decode('utf-8'), parse_float=Decimal)
 
+        return None
+
     def run_test(self):
         self.url = urllib.parse.urlparse(self.nodes[0].url)
         self.wallet = MiniWallet(self.nodes[0])
@@ -213,12 +223,12 @@ class RESTTest (BitcoinTestFramework):
         bb_hash = self.nodes[0].getbestblockhash()
 
         # Check result if block does not exists
-        assert_equal(self.test_rest_request(f"/headers/1/{UNKNOWN_PARAM}"), [])
+        assert_equal(self.test_rest_request(f"/headers/{UNKNOWN_PARAM}", query_params={"count": 1}), [])
         self.test_rest_request(f"/block/{UNKNOWN_PARAM}", status=404, ret_type=RetType.OBJ)
 
         # Check result if block is not in the active chain
         self.nodes[0].invalidateblock(bb_hash)
-        assert_equal(self.test_rest_request(f'/headers/1/{bb_hash}'), [])
+        assert_equal(self.test_rest_request(f'/headers/{bb_hash}', query_params={'count': 1}), [])
         self.test_rest_request(f'/block/{bb_hash}')
         self.nodes[0].reconsiderblock(bb_hash)
 
@@ -228,7 +238,7 @@ class RESTTest (BitcoinTestFramework):
         response_bytes = response.read()
 
         # Compare with block header
-        response_header = self.test_rest_request(f"/headers/1/{bb_hash}", req_type=ReqType.BIN, ret_type=RetType.OBJ)
+        response_header = self.test_rest_request(f"/headers/{bb_hash}", req_type=ReqType.BIN, ret_type=RetType.OBJ, query_params={"count": 1})
         assert_equal(int(response_header.getheader('content-length')), BLOCK_HEADER_SIZE)
         response_header_bytes = response_header.read()
         assert_equal(response_bytes[:BLOCK_HEADER_SIZE], response_header_bytes)
@@ -240,7 +250,7 @@ class RESTTest (BitcoinTestFramework):
         assert_equal(response_bytes.hex().encode(), response_hex_bytes)
 
         # Compare with hex block header
-        response_header_hex = self.test_rest_request(f"/headers/1/{bb_hash}", req_type=ReqType.HEX, ret_type=RetType.OBJ)
+        response_header_hex = self.test_rest_request(f"/headers/{bb_hash}", req_type=ReqType.HEX, ret_type=RetType.OBJ, query_params={"count": 1})
         assert_greater_than(int(response_header_hex.getheader('content-length')), BLOCK_HEADER_SIZE*2)
         response_header_hex_bytes = response_header_hex.read(BLOCK_HEADER_SIZE*2)
         assert_equal(response_bytes[:BLOCK_HEADER_SIZE].hex().encode(), response_header_hex_bytes)
@@ -267,7 +277,7 @@ class RESTTest (BitcoinTestFramework):
         self.test_rest_request("/blockhashbyheight/", ret_type=RetType.OBJ, status=400)
 
         # Compare with json block header
-        json_obj = self.test_rest_request(f"/headers/1/{bb_hash}")
+        json_obj = self.test_rest_request(f"/headers/{bb_hash}", query_params={"count": 1})
         assert_equal(len(json_obj), 1)  # ensure that there is one header in the json response
         assert_equal(json_obj[0]['hash'], bb_hash)  # request/response hash should be the same
 
@@ -278,9 +288,9 @@ class RESTTest (BitcoinTestFramework):
 
         # See if we can get 5 headers in one response
         self.generate(self.nodes[1], 5)
-        json_obj = self.test_rest_request(f"/headers/5/{bb_hash}")
+        json_obj = self.test_rest_request(f"/headers/{bb_hash}", query_params={"count": 5})
         assert_equal(len(json_obj), 5)  # now we should have 5 header objects
-        json_obj = self.test_rest_request(f"/blockfilterheaders/basic/5/{bb_hash}")
+        json_obj = self.test_rest_request(f"/blockfilterheaders/basic/{bb_hash}", query_params={"count": 5})
         first_filter_header = json_obj[0]
         assert_equal(len(json_obj), 5)  # now we should have 5 filter header objects
         json_obj = self.test_rest_request(f"/blockfilter/basic/{bb_hash}")
@@ -294,7 +304,7 @@ class RESTTest (BitcoinTestFramework):
         for num in ['5a', '-5', '0', '2001', '99999999999999999999999999999999999']:
             assert_equal(
                 bytes(f'Header count is invalid or out of acceptable range (1-2000): {num}\r\n', 'ascii'),
-                self.test_rest_request(f"/headers/{num}/{bb_hash}", ret_type=RetType.BYTES, status=400),
+                self.test_rest_request(f"/headers/{bb_hash}", ret_type=RetType.BYTES, status=400, query_params={"count": num}),
             )
 
         self.log.info("Test tx inclusion in the /mempool and /block URIs")
@@ -351,6 +361,11 @@ class RESTTest (BitcoinTestFramework):
         json_obj = self.test_rest_request("/chaininfo")
         assert_equal(json_obj['bestblockhash'], bb_hash)
 
+        # Test compatibility of deprecated and newer endpoints
+        self.log.info("Test compatibility of deprecated and newer endpoints")
+        assert_equal(self.test_rest_request(f"/headers/{bb_hash}", query_params={"count": 1}), self.test_rest_request(f"/headers/1/{bb_hash}"))
+        assert_equal(self.test_rest_request(f"/blockfilterheaders/basic/{bb_hash}", query_params={"count": 1}), self.test_rest_request(f"/blockfilterheaders/basic/5/{bb_hash}"))
+
 
 if __name__ == '__main__':
     RESTTest().main()