Repository: mirumee/ariadne
Pull request #1049 (2023-03-15 16:33:52)

Fix federation support for directives without surrounding whitespace
As written, this regex requires two delimiters before a `directive @custom on FIELD` definition. In some cases (for example, when the preceding line is a comment), this causes `purge_schema_directives` to remove only the `@custom` part, leaving `directive on FIELD` which causes downstream parsing errors.

The solution is to require the second delimiter only if the description is also present.

rafalp: I am wondering if this isn't the time to stop using the regexes spagetti and build clean solution on top of `parse` and GraphQL AST?
rafalp: Linters are failing for this change.
danring: I split the long line, so linters are passing now.
rafalp: Can you add unit test for the issue you are fixing?
danring: There is already a test added [here](https://github.com/mirumee/ariadne/pull/1049/files#diff-38572c5999e8f660100ce4742b3947b3fc0d015487579cadfe40a0835cba33d9R124) for the specific case this fixes -- do you want an additional test somewhere else?
rafalp: I'm blind, sorry 😅

Final thing would be updating changelog to include a note about the issue being fixed.
danring: Should I also bump to 18.2? Or just add a new "since last release" section in the changelog?
rafalp: @danring there's already "next release" section. If you don't see it in your files you'll have to rebase.
danring: Rebased and added an entry to CHANGELOG
codecov[bot]: ## [Codecov](https://codecov.io/gh/mirumee/ariadne/pull/1049?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mirumee) Report
> Merging [#1049](https://codecov.io/gh/mirumee/ariadne/pull/1049?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mirumee) (90d1e43) into [master](https://codecov.io/gh/mirumee/ariadne/commit/2b5f184a054ab7deca6557d0dfbd053f64ef933f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mirumee) (2b5f184) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

```diff
@@           Coverage Diff           @@
##           master    #1049   +/-   ##
=======================================
  Coverage   98.09%   98.09%           
=======================================
  Files         112      112           
  Lines        7252     7256    +4     
=======================================
+ Hits         7114     7118    +4     
  Misses        138      138           
```


| [Impacted Files](https://codecov.io/gh/mirumee/ariadne/pull/1049?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mirumee) | Coverage Δ | |
|---|---|---|
| [ariadne/contrib/federation/utils.py](https://codecov.io/gh/mirumee/ariadne/pull/1049?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mirumee#diff-YXJpYWRuZS9jb250cmliL2ZlZGVyYXRpb24vdXRpbHMucHk=) | `100.00% <ø> (ø)` | |
| [ariadne/contrib/federation/schema.py](https://codecov.io/gh/mirumee/ariadne/pull/1049?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mirumee#diff-YXJpYWRuZS9jb250cmliL2ZlZGVyYXRpb24vc2NoZW1hLnB5) | `100.00% <100.00%> (ø)` | |
| [tests/federation/test\_utils.py](https://codecov.io/gh/mirumee/ariadne/pull/1049?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mirumee#diff-dGVzdHMvZmVkZXJhdGlvbi90ZXN0X3V0aWxzLnB5) | `100.00% <100.00%> (ø)` | |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mirumee)

rafalp: Please rebase
danring: Done

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1b3e5c5..6ff1dd3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@
 - Added support for passing `Enum` types directly to `make_executable_schema`
 - Added `convert_names_case` option to `make_federated_schema`.
 - Added support for the `@interfaceObject` directive in Apollo Federation.
+- Fixed federation support for directives without surrounding whitespace.
 
 
 ## 0.18.1 (2023-02-22)
diff --git a/ariadne/contrib/federation/utils.py b/ariadne/contrib/federation/utils.py
index 9188053..6a0f483 100644
--- a/ariadne/contrib/federation/utils.py
+++ b/ariadne/contrib/federation/utils.py
@@ -23,8 +23,9 @@ _i_token_description_single_line = r"(?:\"(?:[^\"\n\r])*?\")"
 
 _r_directive_definition = re.compile(
     "("
-    f"{_i_token_delimiter}"
-    f"(?:{_i_token_description_block_string}|{_i_token_description_single_line})??"
+    f"(?:{_i_token_delimiter}(?:"
+    f"{_i_token_description_block_string}|{_i_token_description_single_line}"
+    "))??"
     f"{_i_token_delimiter}directive"
     f"(?:{_i_token_delimiter})?@({_i_token_name})"
     f"(?:(?:{_i_token_delimiter})?{_i_token_arguments})?"


Test patch:
diff --git a/tests/federation/test_utils.py b/tests/federation/test_utils.py
index 851ccc7..d6ef51e 100644
--- a/tests/federation/test_utils.py
+++ b/tests/federation/test_utils.py
@@ -121,6 +121,12 @@ def test_purge_directives_remove_custom_directives_with_single_line_description(
     )
 
 
+def test_purge_directives_without_leading_whitespace():
+    type_defs = "#\ndirective @custom on FIELD"
+
+    assert sic(purge_schema_directives(type_defs)) == ""
+
+
 def test_get_entity_types_with_key_directive():
     type_defs = """
         type Query {