fix: allow adding properties starting with 'set'

[supresys]

Currently, valid properties with the 'set' prefix are removed from the schema: settled, settings, setup, setOfParameters, etc. Also, all setters are cut out despite the presence of an explicit 'Field' indication. This is clearly incorrect behavior.

This PR shouldn't break backwards compatibility because it requires explicitly specifying 'Field' attribute.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.81%. Comparing base (53f9d49) to head (57b3a26).
⚠️ Report is 131 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #779      +/-   ##
============================================
- Coverage     95.72%   94.81%   -0.92%     
- Complexity     1773     1846      +73     
============================================
  Files           154      175      +21     
  Lines          4586     4878     +292     
============================================
+ Hits           4390     4625     +235     
- Misses          196      253      +57     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[oojacoboo]

GraphQLite attempts to handle getters and settings by stripping get and set off the method name, and using the remaining for the GraphQL field. You're saying with this PR, you'll specify:

#[Field(name: 'settings')]

And that this isn't currently working? Can you please provide a test case for this.

[supresys]

I'll try to describe it in more detail. The NameStrategy class is responsible for removing 'set' and 'get', so there are no problems there. I am describing the problem of illegitimate removal of fields from a schema.

#[GQL\Type(name: 'Example')]
final readonly class Example
{
    /**
     * ID of the object.
     */
    #[GQL\Field(outputType: 'ID!')]
    public function id(): string
    {
        return 'id';
    }

    /**
     * The main settings.
     */
    #[GQL\Field]
    public function settings(): string
    {
        return 'settings';
    }

    /**
     * Number of menu positions.
     */
    #[GQL\Field(name: 'sushiSet')]
    public function setOfSushi(): int
    {
        return 4;
    }

    /**
     * Is this object settled.
     */
    #[GQL\Field(name: 'isSettled')]
    public function settled(): bool
    {
        return false;
    }

    /**
     * This is legal setter that I want to be in schema.
     */
    #[GQL\Field(name: 'name')]
    public function setName(string $name): string
    {
        return $name;
    }

    /**
     * This one too.
     */
    #[GQL\Field(name: 'setAge')]
    public function setAge(int $age): int
    {
        return $age;
    }
}

This type will be represented in the schema as follows:

type Example {
  "ID of the object."
  id: ID!
}

All fields except 'id' will be cut from the schema, since they contain 'set' prefix in their method name. This is especially bad when we use Doctrine entities as a GraphQL data provider, where renaming class methods is a bad idea. This PR fixes this behavior. After this PR the schema will look as expected:

type Example {
  "ID of the object."
  id: ID!

  "Is this object settled."
  isSettled: Boolean!

  "This is legal setter that I want to be in schema."
  name(name: String!): String!

  "This one too."
  setAge(age: Int!): Int!

  "The main settings."
  settings: String!

  "Number of menu positions."
  sushiSet: Int!
}

And since fields require explicit annotation with the field attribute, backward compatibility issues are not expected. The schema may only contain fields that the developer expects to see.

[oojacoboo]

Thanks. So, why not just check to see if the name param has been defined on the Field attribute, explicitly, and use it if so, instead of checking to see if the method name starts with set?

[supresys]

So, why not just check to see if the name param has been defined on the Field attribute, explicitly, and use it if so, instead of checking to see if the method name starts with set?

I didn't quite understand the question. I'm not adding this check, I'm removing it! The question should be addressed to whoever added this check to the code that's causing the fields to disappear from the schema. This behavior was introduced on 2022/06/11 as part of commit 8078bc2, co-authored by you. I completely agree that the name should be taken from the 'name' property. My PR enables this behaviour. Without these changes, the fields are completely ignored and dropped from the schema, whether 'name' is specified or not. If you don't believe this, just try adding a 'settings' or 'setup' method to any type and set any 'name' and it won't appear in the schema.

#[GQL\Field(name: 'anynameyouwouldlike')]
public function settings(): string
{
    return 'I will not appear in the scheme at all!';
}

[oojacoboo]

@supresys thanks for the clarification. I've tested the issue you've described here and can confirm. We need test coverage for this though. If you could please add a test, we'll get this merged.

Fixes #652 as well I believe.

[supresys]

Fixes #652 as well I believe.

Yes, that is indeed true.

[supresys]

I've added a test that covers this issue and its solution.

[oojacoboo]

Thanks @supresys