Merge 'seastar-json2code: fix the path param handling' from Kefu Chai

this change addresses a regression introduced by
3f534c7, which failed to differentiate the path param from the non-path param, and always prefixed the param with "/".

Fixes 3f534c7
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

Closes #2223

* github.com:scylladb/seastar:
  tests/unit: add a unit test for json2code
  seastar-json2code: fix the path param handling

scripts/seastar-json2code.py

@@ -323,13 +323,14 @@ def add_operation(hfile, ccfile, path, oper):
         else:
             fprint(ccfile, "\n,")
         if is_url:
+            path_param = f"/{path_param}"
             component_type = 'FIXED_STRING'
         elif get_parameter_by_name(oper, path_param).get("allowMultiple",
                                                          False):
             component_type = 'PARAM_UNTIL_END_OF_PATH'
         else:
             component_type = 'PARAM'
-        fprint(ccfile, f'{{"/{path_param}", path_description::url_component_type::{component_type}}}')
+        fprint(ccfile, f'{{"{path_param}", path_description::url_component_type::{component_type}}}')
     fprint(ccfile, '}')
     fprint(ccfile, ',{')
     enum_definitions = ""

tests/unit/CMakeLists.txt

@@ -730,3 +730,30 @@ seastar_add_test (pipe
 
 seastar_add_test (spawn
   SOURCES spawn_test.cc)
+
+seastar_generate_swagger (
+  TARGET rest_api_httpd_swagger
+  VAR rest_api_httpd_swagger_files
+  IN_FILE ${CMAKE_CURRENT_SOURCE_DIR}/api.json
+  OUT_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
+add_executable (rest_api_httpd
+  ${rest_api_httpd_swagger_files}
+  rest_api_httpd.cc)
+target_link_libraries (rest_api_httpd
+  PRIVATE seastar_private)
+target_include_directories (rest_api_httpd
+  PRIVATE ${CMAKE_CURRENT_BINARY_DIR})
+
+add_dependencies (rest_api_httpd rest_api_httpd_swagger)
+add_dependencies (unit_tests rest_api_httpd)
+add_custom_target (test_unit_json2code_run
+  COMMAND ${CMAKE_COMMAND} -E env ${Seastar_TEST_ENVIRONMENT} ${CMAKE_CURRENT_SOURCE_DIR}/json2code_test.py --rest-api-httpd $<TARGET_FILE:rest_api_httpd>
+  USES_TERMINAL)
+add_dependencies (test_unit_json2code_run rest_api_httpd)
+add_test (
+  NAME Seastar.unit.json2code
+  COMMAND ${CMAKE_COMMAND} --build ${Seastar_BINARY_DIR} --target test_unit_json2code_run)
+set_tests_properties (Seastar.unit.json2code
+  PROPERTIES
+    TIMEOUT ${Seastar_TEST_TIMEOUT})

tests/unit/api.json

@@ -0,0 +1,81 @@
+{
+  "apiVersion": "0.0.1",
+  "swaggerVersion": "1.2",
+  "basePath": "{{Protocol}}://{{Host}}",
+  "resourcePath": "/hello",
+  "produces": [
+    "application/json"
+  ],
+  "apis": [
+    {
+      "path": "/hello/world/{var1}/{var2}",
+      "operations": [
+        {
+          "method": "GET",
+          "summary": "Returns the number of seconds since the system was booted",
+          "type": "long",
+          "nickname": "hello_world",
+          "produces": [
+            "application/json"
+          ],
+          "parameters": [
+            {
+              "name": "var2",
+              "description": "Full path of file or directory",
+              "required": true,
+              "allowMultiple": true,
+              "type": "string",
+              "paramType": "path"
+            },
+            {
+              "name": "var1",
+              "description": "Full path of file or directory",
+              "required": true,
+              "allowMultiple": false,
+              "type": "string",
+              "paramType": "path"
+            },
+            {
+              "name": "query_enum",
+              "description": "The operation to perform",
+              "required": true,
+              "allowMultiple": false,
+              "type": "string",
+              "paramType": "query",
+              "enum": [
+                "VAL1",
+                "VAL2",
+                "VAL3"
+              ]
+            }
+          ]
+        }
+      ]
+    }
+  ],
+  "models": {
+    "my_object": {
+      "id": "my_object",
+      "description": "Demonstrate an object",
+      "properties": {
+        "var1": {
+          "type": "string",
+          "description": "The first parameter in the path"
+        },
+        "var2": {
+          "type": "string",
+          "description": "The second parameter in the path"
+        },
+        "enum_var": {
+          "type": "string",
+          "description": "Demonstrate an enum returned, note this is not the same enum type of the request",
+          "enum": [
+            "VAL1",
+            "VAL2",
+            "VAL3"
+          ]
+        }
+      }
+    }
+  }
+}

tests/unit/json2code_test.py

@@ -0,0 +1,97 @@
+#!/usr/bin/env python3
+#
+# This file is open source software, licensed to you under the terms
+# of the Apache License, Version 2.0 (the "License").  See the NOTICE file
+# distributed with this work for additional information regarding copyright
+# ownership.  You may not use this file except in compliance with the License.
+#
+# You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+#
+# Copyright (C) 2024 Scylladb, Ltd.
+#
+
+import argparse
+import json
+import subprocess
+import sys
+import unittest
+import urllib.request
+import urllib.parse
+
+
+class TestJson2Code(unittest.TestCase):
+    rest_api_httpd = None
+    server = None
+    port = 10000
+
+    @classmethod
+    def setUpClass(cls):
+        args = [cls.rest_api_httpd, '--port', '10000', '--smp=2']
+        cls.server = subprocess.Popen(args,
+                                      stdin=subprocess.PIPE,
+                                      stdout=subprocess.PIPE,
+                                      bufsize=0, text=True)
+        # wait until the server is ready for serve
+        cls.server.stdout.readline()
+
+    @classmethod
+    def tearDownClass(cls):
+        cls.server.terminate()
+
+    def test_path_params(self):
+        var1 = 'bon'
+        var2 = 'jour'
+        query_enum = 'VAL2'
+        params = urllib.parse.urlencode({'query_enum': query_enum})
+        url = f'http://localhost:{self.port}/hello/world/{var1}/{var2}?{params}'
+        with urllib.request.urlopen(url) as f:
+            response = json.loads(f.read().decode('utf-8'))
+            self.assertEqual(response['var1'], f'/{var1}')
+            self.assertEqual(response['var2'], f'/{var2}')
+            self.assertEqual(response['enum_var'], query_enum)
+
+    def test_bad_enum(self):
+        var1 = 'bon'
+        var2 = 'jour'
+        query_enum = 'unknown value'
+        params = urllib.parse.urlencode({'query_enum': query_enum})
+        url = f'http://localhost:{self.port}/hello/world/{var1}/{var2}?{params}'
+        with urllib.request.urlopen(url) as f:
+            response = json.loads(f.read().decode('utf-8'))
+            self.assertEqual(response['var1'], f'/{var1}')
+            self.assertEqual(response['var2'], f'/{var2}')
+            self.assertEqual(response['enum_var'], 'Unknown')
+
+    def test_missing_path_param(self):
+        query_enum = 'VAL2'
+        params = urllib.parse.urlencode({'query_enum': query_enum})
+        url = f'http://localhost:{self.port}/hello/world/?{params}'
+        with self.assertRaises(urllib.error.HTTPError) as e:
+            with urllib.request.urlopen(url):
+                pass
+            ex = e.exception
+            self.assertEqual(ex.code, 404)
+            response = json.loads(ex.read().decode('utf-8'))
+            self.assertEqual(response['message'], 'Not found')
+            self.assertEqual(response['code'], 404)
+
+
+if __name__ == '__main__':
+    parser = argparse.ArgumentParser()
+    parser.add_argument('--rest-api-httpd',
+                        required=True,
+                        help='Path of the rest_api_httpd executable')
+    opts, remaining = parser.parse_known_args()
+    remaining.insert(0, sys.argv[0])
+    TestJson2Code.rest_api_httpd = opts.rest_api_httpd
+    unittest.main(argv=remaining)

tests/unit/rest_api_httpd.cc

@@ -0,0 +1,86 @@
+/*
+ * This file is open source software, licensed to you under the terms
+ * of the Apache License, Version 2.0 (the "License").  See the NOTICE file
+ * distributed with this work for additional information regarding copyright
+ * ownership.  You may not use this file except in compliance with the License.
+ *
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+/*
+ * Copyright (C) 2024 Scylladb, Ltd.
+ */
+
+#include <memory>
+#include <fmt/core.h>
+
+#include <seastar/http/httpd.hh>
+#include <seastar/http/handlers.hh>
+#include <seastar/http/function_handlers.hh>
+#include <seastar/http/file_handler.hh>
+#include <seastar/core/seastar.hh>
+#include <seastar/core/reactor.hh>
+#include <seastar/http/api_docs.hh>
+#include <seastar/core/thread.hh>
+#include <seastar/net/inet_address.hh>
+#include <seastar/util/defer.hh>
+#include "../../apps/lib/stop_signal.hh"
+#include "api.json.hh"
+
+namespace bpo = boost::program_options;
+
+using namespace seastar;
+using namespace httpd;
+
+void set_routes(routes& r) {
+    api_json::hello_world.set(r, [] (const_req req) {
+        api_json::my_object obj;
+        obj.var1 = req.param.at("var1");
+        obj.var2 = req.param.at("var2");
+        api_json::ns_hello_world::query_enum v = api_json::ns_hello_world::str2query_enum(req.query_parameters.at("query_enum"));
+        // This demonstrate enum conversion
+        obj.enum_var = v;
+        return obj;
+    });
+}
+
+int main(int ac, char** av) {
+    app_template app;
+
+    app.add_options()("port", bpo::value<uint16_t>()->default_value(10000), "HTTP Server port");
+
+    return app.run(ac, av, [&] {
+        return seastar::async([&] {
+            seastar_apps_lib::stop_signal stop_signal;
+            auto&& config = app.configuration();
+            uint16_t port = config["port"].as<uint16_t>();
+            auto server = std::make_unique<http_server_control>();
+            auto rb = make_shared<api_registry_builder>("apps/httpd/");
+            server->start().get();
+
+            auto stop_server = defer([&] () noexcept {
+                std::cout << "Stoppping HTTP server" << std::endl; // This can throw, but won't.
+                server->stop().get();
+            });
+
+            server->set_routes(set_routes).get();
+            server->set_routes([rb](routes& r){rb->set_api_doc(r);}).get();
+            server->set_routes([rb](routes& r) {rb->register_function(r, "demo", "rest api test");}).get();
+            server->listen(port).get();
+
+            fmt::print("{}\n", port);
+            fflush(stdout);
+
+            stop_signal.wait().get();
+            return 0;
+        });
+    });
+}