Skip to content

Commit

Permalink
Merge pull request #151 from evo-company/fix-fragments-merging
Browse files Browse the repository at this point in the history 
[fix] [#149] do not merge/extract fragment fields into node fields
  • Loading branch information
@kindermax
kindermax authored Jun 10, 2024
2 parents 2232618 + 3674817 commit 8b99382
Show file tree
Hide file tree
Showing 3 changed files with 163 additions and 79 deletions.
60 changes: 37 additions & 23 deletions hiku/query.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ def _merge(
link_directives: t.DefaultDict[t.Tuple, t.List] = defaultdict(list)
to_merge = OrderedDict()
fields_iter = chain.from_iterable(e.fields for e in nodes)
fragments_iter = chain.from_iterable(e.fragments for e in nodes)

for field in fields_iter:
key = field_key(field)
Expand All @@ -309,38 +308,53 @@ def _merge(
yield field

if not visited_fields and not to_merge:
for fr in fragments_iter:
for fr in chain.from_iterable(e.fragments for e in nodes):
yield fr
else:
for fr in fragments_iter:
fr_fields = []
for field in fr.node.fields:
key = (field.name, field.options_hash, field.alias)

if field.__class__ is Link:
field = t.cast(Link, field)
if key not in to_merge:
to_merge[key] = [field.node]
links[key] = field
for node in nodes:
for fr in node.fragments:
fr_fields: t.List[FieldOrLink] = []
for field in fr.node.fields:
key = (field.name, field.options_hash, field.alias)

if field.__class__ is Link:
field = t.cast(Link, field)

# If fragment field not exists in node fields, we
# can skip merging it with node fields and just
# leave it in a fragment.
# Field's own node will be merged as usuall
if field.name not in node.fields_map:
fr_fields.append(_merge_link(field))
continue

if key not in to_merge:
to_merge[key] = [field.node]
links[key] = field
else:
to_merge[key].append(field.node)
link_directives[key].extend(field.directives)
else:
to_merge[key].append(field.node)
link_directives[key].extend(field.directives)
else:
if key not in visited_fields:
fr_fields.append(field)

fr_key = (fr.type_name, tuple(field_key(f) for f in fr_fields))
if fr_key not in visited_fragments:
visited_fragments.add(fr_key)
if fr_fields:
yield Fragment(fr.type_name, fr_fields)
if key not in visited_fields:
fr_fields.append(field)

fr_key = (fr.type_name, tuple(field_key(f) for f in fr_fields))
if fr_key not in visited_fragments:
visited_fragments.add(fr_key)
if fr_fields:
yield Fragment(fr.type_name, fr_fields)

for key, values in to_merge.items():
link = links[key]
directives = link_directives[key]
yield link.copy(node=merge(values), directives=tuple(directives))


def _merge_link(link: Link) -> Link:
"""Recursively merge link node fields and return new link"""
return link.copy(node=merge([link.node]))


def merge(nodes: t.Iterable[Node]) -> Node:
"""Merges multiple queries into one query
Expand Down
156 changes: 101 additions & 55 deletions tests/test_read_graphql.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,55 @@ def test_mutation_operation():
)


def test_named_fragments():
def test_named_fragments() -> None:
PinsLink = Link(
"pins",
Node(
[
Field("gunya"),
Link(
"kilned",
Node([Field("rusk")]),
),
],
[],
),
)

SneezerLink = Link(
"sneezer",
Node(
[
Field("flowers"),
Field("apres"),
],
[
Fragment('Makai', [
Field("doozie"),
PinsLink
]),
]
),
options={"gire": "noatak"},
)

GiltsLink = Link(
"gilts",
Node(
[
SneezerLink
],
[
Fragment("Valium", [
Link(
"movies",
Node([Field("boree")]),
),
]),
]
),
)

check_read(
"""
query Juger {
Expand All @@ -200,6 +248,9 @@ def test_named_fragments():
doozie
pins {
gunya
kilned {
rusk
}
...Meer
}
}
Expand All @@ -209,59 +260,7 @@ def test_named_fragments():
}
}
""",
Node(
[
Link(
"gilts",
Node(
[
Link(
"sneezer",
Node(
[
Field("flowers"),
Field("apres"),
Link(
"pins",
Node(
[
Field("gunya"),

Link(
"kilned",
Node(
[
Field("rusk"),
]
),
),
], [],
),
),

], [
Fragment('Makai', [
Field("doozie"),
]),
]
),
options={"gire": "noatak"},
),

Link(
"movies",
Node(
[
Field("boree"),
]
),
),
],
[]
),
),
]
),
Node([GiltsLink]),
)


Expand Down Expand Up @@ -719,7 +718,54 @@ def test_parse_interface_with_one_fragment():
)


def test_merge_node_with_fragment_on_node():
def test_merge_node_with_fragment_on_node() -> None:
check_read(
"""
query GetContext {
context {
user {
id
name
... on User {
id
email
}
}
... on Context {
user {
... on User {
id
email
}
}
}
}
}
""",

Node(
[
Link(
"context",
Node([
Link("user", Node([
Field("id"),
Field("name"),
], [
Fragment('User', [
Field("email"),
]),
])),
], []),
)
]
),
)



def test_merge_fragment_for_union() -> None:
"""We do not know if fragments are for unions or not when we parsing query"""
check_read(
"""
query GetContext {
Expand Down
26 changes: 25 additions & 1 deletion tests/test_union.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ def maybe_get_media():
Node('Audio', [
Field('id', Integer, resolve_audio_fields),
Field('duration', String, resolve_audio_fields),
Link('user', TypeRef['User'], link_user, requires=None),
]),
Node('Video', [
Field('id', Integer, resolve_video_fields),
Expand Down Expand Up @@ -330,7 +331,7 @@ def test_query_only_typename():
}


def test_validate_query_can_not_contain_shared_fields():
def test_validate_query_can_not_contain_shared_fields_in_union():
query = """
query SearchMedia($text: String) {
searchMedia(text: $text) {
Expand Down Expand Up @@ -395,3 +396,26 @@ def test_validate_union_type_field_has_no_such_option():
assert errors == [
'Unknown options for "Audio.duration": size',
]


def test_validate_query_can_contain_shared_links():
# TODO: the problem here is probably because of fragment merging algorythm
query = """
query SearchMedia($text: String) {
searchMedia(text: $text) {
__typename
... on Audio {
duration
user {
id
}
}
... on Video {
thumbnailUrl(size: 100)
}
}
}
"""

errors = validate(GRAPH, read(query, {'text': 'foo'}))
assert not errors

0 comments on commit 8b99382

Please sign in to comment.