feat: added object_type_name_prefix

[superlevure]

This MR adds the ability to pass object_type_name_prefix: string to Schema, the prefix will be prepended to:

• Queries name
• Mutations name
• Subscription name
• Types name

Fields are left unmodified.

This is useful in micro-services architecture to add the service's name in the Schema.

With

schema = Schema(
    Query,
    Mutation,
    Subscription,
    auto_camelcase=True,
    object_type_name_prefix="prefix",
)

The schema:

type Query {
    inner: MyOtherType
}

type MyOtherType {
    field: String
    myType: MyType
}

type MyType {
    field: String
}

type Mutation {
    createUser(name: String): CreateUser
}

type CreateUser {
    name: String
}

type Subscription {
    countToTen: Int
}

Becomes

schema {
    query: PrefixQuery
    mutation: PrefixMutation
    subscription: PrefixSubscription
}

type PrefixQuery {
    prefixInner: PrefixMyOtherType
}

type PrefixMyOtherType {
    field: String
    myType: PrefixMyType
}

type PrefixMyType {
    field: String
}

type PrefixMutation {
    prefixCreateUser(name: String): PrefixCreateUser
}

type PrefixCreateUser {
    name: String
}

type PrefixSubscription {
    prefixCountToTen: Int
}

[codecov[bot]]

Codecov Report

Base: 95.87% // Head: 95.94% // Increases project coverage by +0.07% :tada:

Coverage data is based on head (0ea4636) compared to base (6969023). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1466      +/-   ##
==========================================
+ Coverage   95.87%   95.94%   +0.07%     
==========================================
  Files          50       50              
  Lines        1722     1753      +31     
==========================================
+ Hits         1651     1682      +31     
  Misses         71       71              

Impacted Files	Coverage Δ
graphene/types/base.py	96.96% <100.00%> (+0.09%)	:arrow_up:
graphene/types/schema.py	95.73% <100.00%> (+0.41%)	:arrow_up:
graphene/types/utils.py	89.18% <100.00%> (+1.68%)	:arrow_up:
graphene/types/enum.py	100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[erikwrede]

Thanks for the PR, this looks helpful! Will test it out locally tomorrow.

[erikwrede]

Quick comment which is not a full review: This also prefixes the root type names. I'm wondering if this is really desirable, as Query, Mutation and Subscription types are basically the standard/usual names. The prefixes will cause Docs to deviate from the usual pattern. Maybe a way to edit this behavior would be useful.

My intuition is that excluding root types from prefixing actually helps with schema stitching and general readability of a schema. Happy to have my mind changed on this one though, I'm open to discuss it if you have any compelling reasons! 🙂

[superlevure]

Hi @erikwrede

Thanks for the review! I'm glad you think that it could be useful to Graphene.

Quick comment which is not a full review: This also prefixes the root type names. I'm wondering if this is really desirable, as Query, Mutation and Subscription types are basically the standard/usual names. The prefixes will cause Docs to deviate from the usual pattern. Maybe a way to edit this behavior would be useful.

My intuition is that excluding root types from prefixing actually helps with schema stitching and general readability of a schema. Happy to have my mind changed on this one though, I'm open to discuss it if you have any compelling reasons! 🙂

Absolutely, I was not sure about this one but I agree. I'll push a fix

Is there a reason we are only prefixing ObjectTypes, but excluding things like Enums, I terfaces and Unions? Especially in a Microservice scenario like the one you mentioned these would most likely also be specific to the Microservice. The parameter name sort of specifies that the prefix is only applicable to ObjectTypes, but I could imagine this restriction to be confusing for other devs.

I missed that indeed, we should apply the prefix to those too.

Can we add a test case for the Field("Typename-As-import-String") notation? Just want to make sure future changes to that syntax aren't broken by prefixes. This might be the case if we ever decide to allow Field("Typename") using the schema as a registry instead of import strings.

Sure thing!

[superlevure]

I've updated the PR to address your comments @erikwrede:

• Root type names are left untouched
• Prefix is now added to the name of:
  • ObjectType
  • InputType
  • Interface
  • Union
  • Enum
  • Query / Mutation / Subscription
• Added a test for the Field("Typename-As-import-String") notation
• Updated documentation

Let me know what you think :)

[superlevure]

I've pushed some changes to add the ability to override type_name_prefix for specific types @tcleonard @erikwrede

I realized that this is needed when defining external GraphQL Types in a federated Schema to avoid ending up with ServiceAServiceBTypeC

Let me know what you think :)

[erikwrede]

@superlevure will test this in combination with some graphene federation v2.0 support reviews (https://github.com/graphql-python/graphene-federation/pull/4 ) over the weekend, because that's probably the main use case for this feature. Going to make sure we haven't created any incompatibilities by accident.

[superlevure]

@superlevure will test this in combination with some graphene federation v2.0 support reviews (graphql-python/graphene-federation#4 ) over the weekend, because that's probably the main use case for this feature. Going to make sure we haven't created any incompatibilities by accident.

Thank you! 🙏

Let me know if I can do anything to help

[erikwrede]

@superlevure if you have experience using federation, then trying out the PR I mentioned would definitely be very useful 😊

[erikwrede]

Hey @superlevure any updates? Would love to get this merged soon 😊

[superlevure]

Hey @superlevure any updates? Would love to get this merged soon 😊

Hey @erikwrede, sorry I haven't had the chance to get back at it yet; it is still on my radar though