From e8b942b5c690b404ee81058a37dc9f6b32790fe3 Mon Sep 17 00:00:00 2001 From: Neo Ko Date: Mon, 11 May 2026 14:58:10 +0800 Subject: [PATCH 1/3] fix: can't query vertex with number vertex id 1. make vertex api compactible with number type vertex id 2. add test case for add/append/eliminate/remove vertex with number type vertex id --- hugegraph-python-client/pyproject.toml | 2 +- .../src/pyhugegraph/api/graph.py | 14 +++++----- .../src/pyhugegraph/utils/huge_router.py | 9 +++++++ .../src/tests/api/test_graph.py | 27 +++++++++++++++++++ .../src/tests/client_utils.py | 5 ++++ 5 files changed, 50 insertions(+), 7 deletions(-) diff --git a/hugegraph-python-client/pyproject.toml b/hugegraph-python-client/pyproject.toml index 2c847090e..5200a58fb 100644 --- a/hugegraph-python-client/pyproject.toml +++ b/hugegraph-python-client/pyproject.toml @@ -32,7 +32,7 @@ dependencies = [ "requests", "setuptools", "urllib3", - "rich", + "rich" ] [project.urls] diff --git a/hugegraph-python-client/src/pyhugegraph/api/graph.py b/hugegraph-python-client/src/pyhugegraph/api/graph.py index 4b6aab1c0..c021f619b 100644 --- a/hugegraph-python-client/src/pyhugegraph/api/graph.py +++ b/hugegraph-python-client/src/pyhugegraph/api/graph.py @@ -45,21 +45,21 @@ def addVertices(self, input_data): return [VertexData({"id": item}) for item in response] return None - @router.http("PUT", 'graph/vertices/"{vertex_id}"?action=append') + @router.http("PUT", 'graph/vertices/{vertex_id}?action=append') def appendVertex(self, vertex_id, properties): # pylint: disable=unused-argument data = {"properties": properties} if response := self._invoke_request(data=json.dumps(data)): return VertexData(response) return None - @router.http("PUT", 'graph/vertices/"{vertex_id}"?action=eliminate') + @router.http("PUT", 'graph/vertices/{vertex_id}?action=eliminate') def eliminateVertex(self, vertex_id, properties): # pylint: disable=unused-argument data = {"properties": properties} if response := self._invoke_request(data=json.dumps(data)): return VertexData(response) return None - @router.http("GET", 'graph/vertices/"{vertex_id}"') + @router.http("GET", 'graph/vertices/{vertex_id}') def getVertexById(self, vertex_id): # pylint: disable=unused-argument if response := self._invoke_request(): return VertexData(response) @@ -101,7 +101,7 @@ def getVertexByCondition(self, label="", limit=0, page=None, properties=None): return [VertexData(item) for item in response["vertices"]] return None - @router.http("DELETE", 'graph/vertices/"{vertex_id}"') + @router.http("DELETE", 'graph/vertices/{vertex_id}') def removeVertexById(self, vertex_id): # pylint: disable=unused-argument return self._invoke_request() @@ -200,8 +200,10 @@ def getVerticesById(self, vertex_ids) -> list[VertexData] | None: if not vertex_ids: return [] path = "traversers/vertices?" - for vertex_id in vertex_ids: - path += f'ids="{vertex_id}"&' # pylint: disable=consider-using-join + quoted_vertex_ids = map(json.dumps, vertex_ids) + + for vertex_id in quoted_vertex_ids: + path += f'ids={vertex_id}&' # pylint: disable=consider-using-join path = path.rstrip("&") if response := self._sess.request(path): return [VertexData(item) for item in response["vertices"]] diff --git a/hugegraph-python-client/src/pyhugegraph/utils/huge_router.py b/hugegraph-python-client/src/pyhugegraph/utils/huge_router.py index 580f78f07..8ecfc93a0 100644 --- a/hugegraph-python-client/src/pyhugegraph/utils/huge_router.py +++ b/hugegraph-python-client/src/pyhugegraph/utils/huge_router.py @@ -17,6 +17,7 @@ import functools import inspect +import json import re import threading from collections.abc import Callable @@ -148,6 +149,14 @@ def wrapper(self: "HGraphContext", *args: Any, **kwargs: Any) -> Any: all_kwargs["graphspace"] = graphspace_arg or graphspace_cfg formatted_path = path.format(**all_kwargs) + elif "{vertex_id}" in path: + # fix vertex_id format process + # only quote string type vertex_id + vertex_id = all_kwargs.pop("vertex_id") + if isinstance(vertex_id, str): + vertex_id = "\"" + vertex_id + "\"" + all_kwargs['vertex_id'] = vertex_id + formatted_path = path.format(**all_kwargs) else: formatted_path = path.format(**all_kwargs) else: diff --git a/hugegraph-python-client/src/tests/api/test_graph.py b/hugegraph-python-client/src/tests/api/test_graph.py index a66c3fb9f..279be058d 100644 --- a/hugegraph-python-client/src/tests/api/test_graph.py +++ b/hugegraph-python-client/src/tests/api/test_graph.py @@ -22,6 +22,7 @@ from ..client_utils import ClientUtils + class TestGraphManager(unittest.TestCase): client = None graph = None @@ -57,17 +58,33 @@ def test_append_vertex(self): appended_vertex = self.graph.appendVertex(vertex.id, {"city": "Beijing"}) self.assertEqual(appended_vertex.properties["city"], "Beijing") + def test_append_vertex_with_number_id(self): + vertex = self.graph.addVertex("department", {"name": "DepartmentA", "headcount": 10, "floor": 1}) + appended_vertex = self.graph.appendVertex(vertex.id, {"headcount": 15}) + self.assertEqual(appended_vertex.properties["headcount"], 15) + def test_eliminate_vertex(self): vertex = self.graph.addVertex("person", {"name": "marko", "age": 29, "city": "Beijing"}) self.graph.eliminateVertex(vertex.id, {"city": "Beijing"}) eliminated_vertex = self.graph.getVertexById(vertex.id) self.assertIsNone(eliminated_vertex.properties.get("city")) + def test_eliminate_vertex_with_number_id(self): + vertex = self.graph.addVertex("department", {"name": "DepartmentA", "headcount": 10, "floor": 1}) + self.graph.eliminateVertex(vertex.id, {"floor": 1}) + eliminated_vertex = self.graph.getVertexById(vertex.id) + self.assertIsNone(eliminated_vertex.properties.get("floor")) + def test_get_vertex_by_id(self): vertex = self.graph.addVertex("person", {"name": "Alice", "age": 20}) retrieved_vertex = self.graph.getVertexById(vertex.id) self.assertEqual(retrieved_vertex.id, vertex.id) + def test_get_vertex_by_number_id(self): + vertex = self.graph.addVertex("department", {"name": "DepartmentA", "headcount": 10, "floor": 1}) + retrieved_vertex = self.graph.getVertexById(vertex.id) + self.assertEqual(retrieved_vertex.id, vertex.id) + def test_get_vertex_by_page(self): self.graph.addVertex("person", {"name": "Alice", "age": 20}) self.graph.addVertex("person", {"name": "Bob", "age": 23}) @@ -89,6 +106,16 @@ def test_remove_vertex_by_id(self): except NotFoundError as e: self.assertTrue("Alice\\' does not exist" in str(e)) + def test_remove_vertex_by_number_id(self): + vertex = self.graph.addVertex("department", {"name": "DepartmentA", "headcount": 10, "floor": 1}) + self.graph.removeVertexById(vertex.id) + try: + self.graph.getVertexById(vertex.id) + except NotFoundError as e: + msg = "\\'{}\\' does not exist".format(vertex.id) + logger.info(f'test_msg: {msg}') + self.assertTrue(msg in str(e)) + def test_add_edge(self): vertex1 = self.graph.addVertex("person", {"name": "Alice", "age": 20}) vertex2 = self.graph.addVertex("person", {"name": "Bob", "age": 23}) diff --git a/hugegraph-python-client/src/tests/client_utils.py b/hugegraph-python-client/src/tests/client_utils.py index 1914cdb0c..2c4297222 100644 --- a/hugegraph-python-client/src/tests/client_utils.py +++ b/hugegraph-python-client/src/tests/client_utils.py @@ -56,6 +56,8 @@ def init_property_key(self): schema.propertyKey("date").asDate().ifNotExist().create() schema.propertyKey("price").asInt().ifNotExist().create() schema.propertyKey("weight").asDouble().ifNotExist().create() + schema.propertyKey("headcount").asInt().ifNotExist().create() + schema.propertyKey("floor").asInt().ifNotExist().create() def init_vertex_label(self): schema = self.schema @@ -68,6 +70,9 @@ def init_vertex_label(self): schema.vertexLabel("book").useCustomizeStringId().properties("name", "price").nullableKeys( "price" ).ifNotExist().create() + schema.vertexLabel("department").properties("name", "headcount", "floor").nullableKeys( + "floor" + ).ifNotExist().create() def init_edge_label(self): schema = self.schema From a4dea6597c80e62e6d46a74f6946b92a03de858c Mon Sep 17 00:00:00 2001 From: Neo Ko Date: Wed, 20 May 2026 17:16:57 +0800 Subject: [PATCH 2/3] feat(client): add recursive descent PathParser for route templates Implement a PathParser using recursive descent parsing to correctly handle path parameter substitution with context preservation (e.g., quoted vs unquoted parameters in different positions). Grammar: - path = segment ('/' segment)* query? - param = '{' IDENTIFIER '}' - query_param = key '=' value - value = TEXT | '"' param '"' | param This fixes issues with regex-based parsing when handling complex templates like '{params1}?action="{params2}"' where parameters appear in different contexts. --- .../src/pyhugegraph/utils/path_parser.py | 412 ++++++++++++++++++ .../src/tests/api/test_path_parser.py | 238 ++++++++++ 2 files changed, 650 insertions(+) create mode 100644 hugegraph-python-client/src/pyhugegraph/utils/path_parser.py create mode 100644 hugegraph-python-client/src/tests/api/test_path_parser.py diff --git a/hugegraph-python-client/src/pyhugegraph/utils/path_parser.py b/hugegraph-python-client/src/pyhugegraph/utils/path_parser.py new file mode 100644 index 000000000..7c442e54f --- /dev/null +++ b/hugegraph-python-client/src/pyhugegraph/utils/path_parser.py @@ -0,0 +1,412 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); 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. + +"""Path template parser for HTTP route parameter substitution. + +Uses recursive descent parsing to correctly handle parameter positions +and preserve context (e.g., quoted vs unquoted parameters). +""" + +from collections.abc import Iterator +from dataclasses import dataclass, field +from enum import Enum, auto +from typing import Any + + +class TokenType(Enum): + TEXT = auto() # Plain text segment + LBRACE = auto() # '{' + RBRACE = auto() # '}' + DQUOTE = auto() # '"' + SLASH = auto() # '/' + QUESTION = auto() # '?' + AMPERSAND = auto() # '&' + EQUALS = auto() # '=' + IDENTIFIER = auto() # Parameter name inside braces + EOF = auto() # End of input + + +@dataclass +class Token: + type: TokenType + value: str + pos: int # Position in source string + + +class Lexer: + """Tokenize path template string.""" + + def __init__(self, source: str): + self._source = source + self._pos = 0 + self._length = len(source) + + def _current(self) -> str | None: + if self._pos >= self._length: + return None + return self._source[self._pos] + + def _advance(self) -> str | None: + char = self._current() + self._pos += 1 + return char + + def _peek_identifier(self) -> bool: + """Check if current position starts an identifier (word chars only).""" + char = self._current() + return char is not None and (char.isalnum() or char in ('_', '-')) + + def _read_identifier(self) -> str: + """Read identifier characters (alphanumeric, underscore, hyphen).""" + start = self._pos + char = self._current() + while char is not None and (char.isalnum() or char in ('_', '-')): + self._pos += 1 + char = self._current() + return self._source[start:self._pos] + + def _read_text(self) -> str: + """Read plain text until special character.""" + start = self._pos + while self._current() is not None and self._current() not in ('{', '}', '"', '/', '?', '&', '='): + self._pos += 1 + return self._source[start:self._pos] + + def tokenize(self) -> Iterator[Token]: + """Generate token stream from source.""" + while self._pos < self._length: + char = self._current() + pos = self._pos + + if char == '{': + self._advance() + yield Token(TokenType.LBRACE, '{', pos) + if self._peek_identifier(): + name = self._read_identifier() + yield Token(TokenType.IDENTIFIER, name, pos + 1) + if self._current() == '}': + self._advance() + yield Token(TokenType.RBRACE, '}', self._pos) + elif char == '}': + # Unmatched closing brace - treat as text + text_char = self._advance() + yield Token(TokenType.TEXT, text_char if text_char else '}', pos) + elif char == '"': + self._advance() + yield Token(TokenType.DQUOTE, '"', pos) + elif char == '/': + self._advance() + yield Token(TokenType.SLASH, '/', pos) + elif char == '?': + self._advance() + yield Token(TokenType.QUESTION, '?', pos) + elif char == '&': + self._advance() + yield Token(TokenType.AMPERSAND, '&', pos) + elif char == '=': + self._advance() + yield Token(TokenType.EQUALS, '=', pos) + else: + text = self._read_text() + if text: + yield Token(TokenType.TEXT, text, pos) + + yield Token(TokenType.EOF, '', self._pos) + + +@dataclass +class ParamNode: + """A parameter placeholder node.""" + name: str + quoted: bool = False # Whether wrapped in quotes + pos: int = 0 + + +@dataclass +class TextNode: + """A plain text segment.""" + text: str + pos: int = 0 + + +@dataclass +class QueryParamNode: + """A query parameter (key=value).""" + key: str + value_nodes: list[Any] = field(default_factory=list) # TextNode or ParamNode + pos: int = 0 + + +@dataclass +class PathNode: + """Root node containing path segments and optional query params.""" + segments: list[Any] = field(default_factory=list) # TextNode, ParamNode, or QueryParamNode + original: str = "" + + +class PathParser: + """ + Parse path templates using recursive descent. + + Grammar: + path = segment ('/' segment)* query? + segment = TEXT | param + param = '{' IDENTIFIER '}' + query = '?' query_param ('&' query_param)* + query_param = key '=' value + value = TEXT | '"' param '"' | param + key = TEXT + + Example: + >>> parser = PathParser.parse('graph/vertices/"{vertex_id}"?action=append') + >>> parser.params + ['vertex_id'] + >>> parser.original + 'graph/vertices/"{vertex_id}"?action=append' + >>> parser.format({'vertex_id': 'test-123'}) + 'graph/vertices/"test-123"?action=append' + """ + + def __init__(self, source: str): + self._source = source + self._lexer = Lexer(source) + self._tokens = list(self._lexer.tokenize()) + self._pos = 0 + self._ast: PathNode = PathNode(original=source) + self._params: list[ParamNode] = [] + self._parse() + + @classmethod + def parse(cls, path: str) -> "PathParser": + """Create a parser instance from a path template.""" + return cls(path) + + def _current(self) -> Token: + if self._pos >= len(self._tokens): + return self._tokens[-1] # EOF + return self._tokens[self._pos] + + def _peek(self, offset: int = 0) -> Token: + idx = self._pos + offset + if idx >= len(self._tokens): + return self._tokens[-1] + return self._tokens[idx] + + def _advance(self) -> Token: + token = self._current() + self._pos += 1 + return token + + def _expect(self, type_: TokenType) -> Token: + token = self._current() + if token.type != type_: + raise SyntaxError(f"Expected {type_}, got {token.type} at position {token.pos}") + return self._advance() + + def _match(self, type_: TokenType) -> bool: + return self._current().type == type_ + + def _parse(self) -> None: + """Parse the entire path.""" + # Parse path segments until query or EOF + while not self._match(TokenType.EOF) and not self._match(TokenType.QUESTION): + if self._match(TokenType.SLASH): + self._advance() + continue + + node = self._parse_segment() + if node: + self._ast.segments.append(node) + + # Parse query params if present + if self._match(TokenType.QUESTION): + self._advance() + self._parse_query_params() + + def _parse_segment(self) -> TextNode | ParamNode | None: + """Parse a single path segment.""" + token = self._current() + + if token.type == TokenType.TEXT: + self._advance() + return TextNode(text=token.value, pos=token.pos) + + if token.type == TokenType.DQUOTE: + # Quoted content in path segment + self._advance() + node = self._parse_quoted_param() + self._expect(TokenType.DQUOTE) + return node + + if token.type == TokenType.LBRACE: + return self._parse_param() + + return None + + def _parse_param(self) -> ParamNode: + """Parse a parameter: '{' IDENTIFIER '}'.""" + start_pos = self._current().pos + self._expect(TokenType.LBRACE) + name_token = self._expect(TokenType.IDENTIFIER) + self._expect(TokenType.RBRACE) + + node = ParamNode(name=name_token.value, pos=start_pos) + self._params.append(node) + return node + + def _parse_quoted_param(self) -> ParamNode | TextNode: + """Parse a quoted parameter: '"' param '"'.""" + if self._match(TokenType.LBRACE): + node = self._parse_param() + node.quoted = True + return node + + # Not a param inside quotes - treat as text + if self._match(TokenType.TEXT): + token = self._advance() + # Return as a ParamNode-like structure for consistency + return TextNode(text=token.value, pos=token.pos) + + return TextNode(text="", pos=self._current().pos) + + def _parse_query_params(self) -> None: + """Parse query parameters: '?' param ('&' param)*.""" + while not self._match(TokenType.EOF): + if self._match(TokenType.AMPERSAND): + self._advance() + continue + + node = self._parse_query_param() + if node: + self._ast.segments.append(node) + + def _parse_query_param(self) -> QueryParamNode | None: + """Parse a query parameter: key '=' value.""" + start_pos = self._current().pos + + # Parse key + if not self._match(TokenType.TEXT): + return None + + key_token = self._advance() + + # Expect '=' + if not self._match(TokenType.EQUALS): + # Key without value - treat as text segment + self._ast.segments.append(TextNode(text=key_token.value, pos=start_pos)) + return None + + self._advance() # consume '=' + + # Parse value + value_nodes = self._parse_query_value() + + return QueryParamNode(key=key_token.value, value_nodes=value_nodes, pos=start_pos) + + def _parse_query_value(self) -> list[TextNode | ParamNode]: + """Parse query value: TEXT | '"' param '"' | param.""" + nodes: list[TextNode | ParamNode] = [] + + while ( + not self._match(TokenType.EOF) + and not self._match(TokenType.AMPERSAND) + and not self._match(TokenType.QUESTION) + ): + token = self._current() + + if token.type == TokenType.TEXT: + nodes.append(TextNode(text=self._advance().value, pos=token.pos)) + elif token.type == TokenType.DQUOTE: + self._advance() # opening quote + # Check if next is a param + if self._match(TokenType.LBRACE): + node = self._parse_param() + node.quoted = True + nodes.append(node) + elif self._match(TokenType.TEXT): + nodes.append(TextNode(text=self._advance().value, pos=token.pos)) + self._expect(TokenType.DQUOTE) # closing quote + elif token.type == TokenType.LBRACE: + nodes.append(self._parse_param()) + else: + break + + return nodes + + @property + def original(self) -> str: + """Return the original path template.""" + return self._source + + @property + def params(self) -> list[str]: + """Return list of parameter names.""" + return [p.name for p in self._params] + + def format(self, values: dict[str, Any]) -> str: + """Substitute parameters with provided values. + + Args: + values: Dictionary mapping parameter names to values. + + Returns: + Formatted path with parameters substituted. + + Raises: + KeyError: If a required parameter is missing. + """ + result = [] + i = 0 + source = self._source + + # Reconstruct with substituted values + for param in self._params: + if param.name not in values: + raise KeyError(f"Missing parameter: {param.name}") + + # Find the param in source and replace + param_str = "{" + param.name + "}" + if param.quoted: + param_str = '"' + param_str + '"' + + # Find position in remaining source + pos = source.find(param_str, i) + if pos == -1: + # Fallback: just the brace pattern + pos = source.find("{" + param.name + "}", i) + + # Add text before param + result.append(source[i:pos]) + + # Add substituted value + value = str(values[param.name]) + if param.quoted: + result.append('"') + result.append(value) + result.append('"') + else: + result.append(value) + + i = pos + len(param_str) + + # Add remaining text + result.append(source[i:]) + + return "".join(result) + + def __repr__(self) -> str: + return f"PathParser(path={self._source!r}, params={self.params})" diff --git a/hugegraph-python-client/src/tests/api/test_path_parser.py b/hugegraph-python-client/src/tests/api/test_path_parser.py new file mode 100644 index 000000000..36dbf2c87 --- /dev/null +++ b/hugegraph-python-client/src/tests/api/test_path_parser.py @@ -0,0 +1,238 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); 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. + +"""Unit tests for PathParser (recursive descent implementation).""" + +import pytest + +from pyhugegraph.utils.path_parser import ( + Lexer, + ParamNode, + PathParser, + QueryParamNode, + TextNode, + Token, + TokenType, +) + + +class TestLexer: + """Test Lexer tokenization.""" + + def tokenize(self, source: str) -> list[Token]: + lexer = Lexer(source) + return [t for t in lexer.tokenize() if t.type != TokenType.EOF] + + def test_tokenize_simple_path(self): + tokens = self.tokenize("graph/vertices") + assert len(tokens) == 3 + assert tokens[0].type == TokenType.TEXT + assert tokens[1].type == TokenType.SLASH + assert tokens[2].type == TokenType.TEXT + + def test_tokenize_param(self): + tokens = self.tokenize("{vertex_id}") + assert len(tokens) == 3 + assert tokens[0].type == TokenType.LBRACE + assert tokens[1].type == TokenType.IDENTIFIER + assert tokens[1].value == "vertex_id" + assert tokens[2].type == TokenType.RBRACE + + def test_tokenize_quoted_param(self): + tokens = self.tokenize('"{vertex_id}"') + assert len(tokens) == 5 + assert tokens[0].type == TokenType.DQUOTE + assert tokens[1].type == TokenType.LBRACE + assert tokens[2].type == TokenType.IDENTIFIER + assert tokens[3].type == TokenType.RBRACE + assert tokens[4].type == TokenType.DQUOTE + + def test_tokenize_query_params(self): + tokens = self.tokenize("?action=append") + assert tokens[0].type == TokenType.QUESTION + assert tokens[1].type == TokenType.TEXT + assert tokens[2].type == TokenType.EQUALS + assert tokens[3].type == TokenType.TEXT + + def test_tokenize_complex_path(self): + tokens = self.tokenize('graph/vertices/"{vertex_id}"?action="{param2}"') + # Should tokenize: graph / vertices / "{vertex_id}" ? action = "{param2}" + assert any(t.type == TokenType.QUESTION for t in tokens) + assert any(t.type == TokenType.EQUALS for t in tokens) + + +class TestPathParserParse: + """Test PathParser.parse() method.""" + + def test_parse_simple_path(self): + parser = PathParser.parse("graph/vertices") + assert parser.params == [] + assert parser.original == "graph/vertices" + + def test_parse_single_param(self): + parser = PathParser.parse("graph/vertices/{vertex_id}") + assert parser.params == ["vertex_id"] + assert parser.original == "graph/vertices/{vertex_id}" + + def test_parse_quoted_param(self): + parser = PathParser.parse('graph/vertices/"{vertex_id}"') + assert parser.params == ["vertex_id"] + assert parser.original == 'graph/vertices/"{vertex_id}"' + + def test_parse_multiple_params(self): + parser = PathParser.parse('traversers/sameneighbors?vertex="{vertex_id}"&other="{other_id}"') + assert "vertex_id" in parser.params + assert "other_id" in parser.params + assert len(parser.params) == 2 + + def test_parse_mixed_params(self): + parser = PathParser.parse('traversers/kout?source="{source_id}"&max_depth={max_depth}') + assert "source_id" in parser.params + assert "max_depth" in parser.params + assert len(parser.params) == 2 + + def test_parse_complex_query_params(self): + # Key scenario: {params1}?action="{params2}" + parser = PathParser.parse('{params1}?action="{params2}"') + assert parser.params == ["params1", "params2"] + assert len(parser.params) == 2 + + +class TestPathParserOriginal: + """Test PathParser.original property.""" + + def test_original_preserved_simple(self): + path = "graph/vertices" + parser = PathParser(path) + assert parser.original == path + + def test_original_preserved_with_params(self): + path = 'graph/vertices/"{vertex_id}"?action=append' + parser = PathParser(path) + assert parser.original == path + + def test_original_preserved_complex(self): + path = '{params1}?action="{params2}"' + parser = PathParser(path) + assert parser.original == path + + +class TestPathParserFormat: + """Test PathParser.format() method.""" + + def test_format_no_params(self): + parser = PathParser("graph/vertices") + result = parser.format({}) + assert result == "graph/vertices" + + def test_format_single_param_plain(self): + parser = PathParser("graphspaces/{graphspace}/auth/users") + result = parser.format({"graphspace": "test_gs"}) + assert result == "graphspaces/test_gs/auth/users" + + def test_format_single_param_quoted(self): + parser = PathParser('graph/vertices/"{vertex_id}"') + result = parser.format({"vertex_id": "test-123"}) + assert result == 'graph/vertices/"test-123"' + + def test_format_multiple_params(self): + parser = PathParser('traversers/sameneighbors?vertex="{vertex_id}"&other="{other_id}"') + result = parser.format({"vertex_id": "v1", "other_id": "v2"}) + assert result == 'traversers/sameneighbors?vertex="v1"&other="v2"' + + def test_format_mixed_params(self): + parser = PathParser('traversers/kout?source="{source_id}"&max_depth={max_depth}') + result = parser.format({"source_id": "src", "max_depth": 3}) + assert result == 'traversers/kout?source="src"&max_depth=3' + + def test_format_with_query_action(self): + parser = PathParser('graph/vertices/"{vertex_id}"?action=append') + result = parser.format({"vertex_id": "test"}) + assert result == 'graph/vertices/"test"?action=append' + + def test_format_complex_query_params(self): + # Key scenario: {params1}?action="{params2}" + parser = PathParser('{params1}?action="{params2}"') + result = parser.format({"params1": "value1", "params2": "value2"}) + assert result == 'value1?action="value2"' + + def test_format_query_param_with_quoted_value(self): + parser = PathParser('?source="{source_id}"&target="{target_id}"&depth={depth}') + result = parser.format({"source_id": "s1", "target_id": "t1", "depth": 5}) + assert result == '?source="s1"&target="t1"&depth=5' + + def test_format_missing_param_raises_error(self): + parser = PathParser('graph/vertices/"{vertex_id}"') + with pytest.raises(KeyError): + parser.format({}) + + +class TestPathParserParams: + """Test PathParser.params property.""" + + def test_params_returns_copy(self): + parser = PathParser("graph/vertices/{vertex_id}") + params1 = parser.params + params2 = parser.params + assert params1 == params2 + # params is a list of strings, check it's a copy + params1_copy = list(params1) + assert params1_copy == params2 + + def test_params_empty_for_no_placeholders(self): + parser = PathParser("graph/vertices") + assert parser.params == [] + + def test_params_extraction_order(self): + parser = PathParser("{a}/{b}/{c}") + assert parser.params == ["a", "b", "c"] + + def test_params_with_hyphen_in_name(self): + parser = PathParser("{vertex-id}") + assert parser.params == ["vertex-id"] + + +class TestPathParserAST: + """Test AST structure produced by parser.""" + + def test_ast_text_nodes(self): + parser = PathParser("graph/vertices") + # All segments should be TextNode + assert all(isinstance(s, TextNode) for s in parser._ast.segments) + + def test_ast_param_node(self): + parser = PathParser("graph/{vertex_id}") + # Should have TextNode and ParamNode + types = [type(s).__name__ for s in parser._ast.segments] + assert "ParamNode" in types + + def test_ast_quoted_param_node(self): + parser = PathParser('"{vertex_id}"') + # Should have ParamNode with quoted=True + param_nodes = [s for s in parser._ast.segments if isinstance(s, ParamNode)] + assert len(param_nodes) == 1 + assert param_nodes[0].quoted is True + + +class TestPathParserRepr: + """Test PathParser.__repr__() method.""" + + def test_repr_shows_path_and_params(self): + parser = PathParser('graph/vertices/"{vertex_id}"') + repr_str = repr(parser) + assert "graph/vertices" in repr_str + assert "vertex_id" in repr_str \ No newline at end of file From acf47f69563d378eebc124d4a5f9fe1b4569ef1d Mon Sep 17 00:00:00 2001 From: Neo Ko Date: Wed, 20 May 2026 17:25:47 +0800 Subject: [PATCH 3/3] refactor(client): use PathParser in http decorator instead of regex Replace regex-based path parameter detection with recursive descent PathParser in the http decorator and RouterMixin. Changes: - Pre-parse path template at decorator definition time for efficiency - Use PathParser.params to check for parameters instead of regex - Use PathParser.format() for path substitution - vertex_id branch: use quoted_override parameter to control quoting (strings get quotes, numbers don't) - Add quoted_override support to PathParser.format() method - Remove unused json import from huge_router.py --- .../src/pyhugegraph/utils/huge_router.py | 41 +++++++++++-------- .../src/pyhugegraph/utils/path_parser.py | 39 ++++++++++++++---- .../src/tests/api/test_path_parser.py | 19 +++++++++ 3 files changed, 73 insertions(+), 26 deletions(-) diff --git a/hugegraph-python-client/src/pyhugegraph/utils/huge_router.py b/hugegraph-python-client/src/pyhugegraph/utils/huge_router.py index 8ecfc93a0..4a7fe5f67 100644 --- a/hugegraph-python-client/src/pyhugegraph/utils/huge_router.py +++ b/hugegraph-python-client/src/pyhugegraph/utils/huge_router.py @@ -17,14 +17,13 @@ import functools import inspect -import json -import re import threading from collections.abc import Callable from dataclasses import dataclass from typing import TYPE_CHECKING, Any, ClassVar from pyhugegraph.utils.log import log +from pyhugegraph.utils.path_parser import PathParser from pyhugegraph.utils.util import ResponseValidation if TYPE_CHECKING: @@ -101,6 +100,8 @@ def http(method: str, path: str) -> Callable: Returns: Callable: The decorator function. """ + # Pre-parse the path template for efficiency + path_parser = PathParser(path) def decorator(func: Callable) -> Callable: """Decorator function that modifies the original function.""" @@ -119,8 +120,8 @@ def wrapper(self: "HGraphContext", *args: Any, **kwargs: Any) -> Any: Returns: Any: The result of the decorated function. """ - # If the pathinfo contains placeholders, format it with the actual arguments - if re.search(r"{\w+}", path): + # If the path has parameters, format it with actual arguments + if path_parser.params: sig = inspect.signature(func) bound_args = sig.bind(self, *args, **kwargs) bound_args.apply_defaults() @@ -132,7 +133,7 @@ def wrapper(self: "HGraphContext", *args: Any, **kwargs: Any) -> Any: # only mounts UserAPI/AccessAPI/BelongAPI/TargetAPI under # /graphspaces/{graphspace}/auth/..., so we fail fast when the # session lacks one rather than producing an unreachable URL. - if "{graphspace}" in path: + if "graphspace" in path_parser.params: graphspace_arg = all_kwargs.get("graphspace") graphspace_cfg = getattr(self.session.cfg, "graphspace", None) gs_supported = getattr(self.session.cfg, "gs_supported", False) @@ -148,17 +149,16 @@ def wrapper(self: "HGraphContext", *args: Any, **kwargs: Any) -> Any: raise ValueError(f"Expected graphspace-prefixed path, got: {path}") all_kwargs["graphspace"] = graphspace_arg or graphspace_cfg - formatted_path = path.format(**all_kwargs) - elif "{vertex_id}" in path: - # fix vertex_id format process - # only quote string type vertex_id - vertex_id = all_kwargs.pop("vertex_id") - if isinstance(vertex_id, str): - vertex_id = "\"" + vertex_id + "\"" - all_kwargs['vertex_id'] = vertex_id - formatted_path = path.format(**all_kwargs) + formatted_path = path_parser.format(all_kwargs) + elif "vertex_id" in path_parser.params: + # Handle vertex_id quoting: string types need quotes, numbers don't + vertex_id = all_kwargs.get("vertex_id") + is_string = isinstance(vertex_id, str) + formatted_path = path_parser.format( + all_kwargs, quoted_override={"vertex_id": is_string} + ) else: - formatted_path = path.format(**all_kwargs) + formatted_path = path_parser.format(all_kwargs) else: formatted_path = path @@ -175,10 +175,13 @@ def wrapper(self: "HGraphContext", *args: Any, **kwargs: Any) -> Any: class RouterMixin: - def _invoke_request_registered(self, placeholders: dict | None = None, validator=None, **kwargs: Any): + def _invoke_request_registered( + self, placeholders: dict | None = None, validator=None, **kwargs: Any + ): """ Make an HTTP request using the stored partial request function. Args: + placeholders: Dictionary of placeholder values for path formatting. **kwargs (Any): Keyword arguments to be passed to the request function. Returns: Any: The response from the HTTP request. @@ -189,9 +192,11 @@ def _invoke_request_registered(self, placeholders: dict | None = None, validator fname = frame.f_code.co_name route = RouterRegistry().routers.get(f"{self.__class__.__name__}.{fname}") - if re.search(r"{\w+}", route.path): + # Use PathParser to format the path + path_parser = PathParser(route.path) + if path_parser.params: assert placeholders is not None, "Placeholders must be provided" - formatted_path = route.path.format(**placeholders) + formatted_path = path_parser.format(placeholders) else: formatted_path = route.path diff --git a/hugegraph-python-client/src/pyhugegraph/utils/path_parser.py b/hugegraph-python-client/src/pyhugegraph/utils/path_parser.py index 7c442e54f..c64adc7f3 100644 --- a/hugegraph-python-client/src/pyhugegraph/utils/path_parser.py +++ b/hugegraph-python-client/src/pyhugegraph/utils/path_parser.py @@ -357,11 +357,17 @@ def params(self) -> list[str]: """Return list of parameter names.""" return [p.name for p in self._params] - def format(self, values: dict[str, Any]) -> str: + def format( + self, + values: dict[str, Any], + quoted_override: dict[str, bool] | None = None, + ) -> str: """Substitute parameters with provided values. Args: values: Dictionary mapping parameter names to values. + quoted_override: Optional dict to override quoted behavior for specific params. + True = wrap value in quotes, False = no quotes (ignoring template). Returns: Formatted path with parameters substituted. @@ -378,30 +384,47 @@ def format(self, values: dict[str, Any]) -> str: if param.name not in values: raise KeyError(f"Missing parameter: {param.name}") + # Determine if this param should be quoted + should_quote = param.quoted + if quoted_override and param.name in quoted_override: + should_quote = quoted_override[param.name] + # Find the param in source and replace param_str = "{" + param.name + "}" + # Template has quotes around param + template_quoted_str = '"' + param_str + '"' + + # Find position in remaining source based on template structure if param.quoted: - param_str = '"' + param_str + '"' + # Template has quotes, look for quoted pattern first + pos = source.find(template_quoted_str, i) + if pos != -1: + consumed_len = len(template_quoted_str) + else: + # Fallback: look for unquoted pattern + pos = source.find(param_str, i) + consumed_len = len(param_str) + else: + # Template has no quotes around this param + pos = source.find(param_str, i) + consumed_len = len(param_str) - # Find position in remaining source - pos = source.find(param_str, i) if pos == -1: - # Fallback: just the brace pattern - pos = source.find("{" + param.name + "}", i) + raise ValueError(f"Cannot find parameter {param.name} in source path") # Add text before param result.append(source[i:pos]) # Add substituted value value = str(values[param.name]) - if param.quoted: + if should_quote: result.append('"') result.append(value) result.append('"') else: result.append(value) - i = pos + len(param_str) + i = pos + consumed_len # Add remaining text result.append(source[i:]) diff --git a/hugegraph-python-client/src/tests/api/test_path_parser.py b/hugegraph-python-client/src/tests/api/test_path_parser.py index 36dbf2c87..4d02b447c 100644 --- a/hugegraph-python-client/src/tests/api/test_path_parser.py +++ b/hugegraph-python-client/src/tests/api/test_path_parser.py @@ -180,6 +180,25 @@ def test_format_missing_param_raises_error(self): with pytest.raises(KeyError): parser.format({}) + def test_format_quoted_override_keep_quotes(self): + """Override quoted=True to True (explicit keep quotes).""" + parser = PathParser('graph/vertices/"{vertex_id}"') + result = parser.format({"vertex_id": "test"}, quoted_override={"vertex_id": True}) + assert result == 'graph/vertices/"test"' + + def test_format_quoted_override_remove_quotes(self): + """Override quoted=True to False (remove quotes from template).""" + parser = PathParser('graph/vertices/"{vertex_id}"') + # For number type vertex_id, we don't want quotes + result = parser.format({"vertex_id": 123}, quoted_override={"vertex_id": False}) + assert result == 'graph/vertices/123' + + def test_format_quoted_override_add_quotes(self): + """Override quoted=False to True (add quotes even if template has none).""" + parser = PathParser("graph/vertices/{vertex_id}") + result = parser.format({"vertex_id": "test"}, quoted_override={"vertex_id": True}) + assert result == 'graph/vertices/"test"' + class TestPathParserParams: """Test PathParser.params property."""