Add Enum Classes for Enum Properties & Parameters

[chrisguitarguy]

I had some stuff in another PR around enums, but wasn't super happy with it.

This adds an Enums namespace to the generated package and an enum class that's just a bag of constants. Properties that have enum values have an @see thing in the docblock pointing to the class.

One thing that's not great here is that schema references for enums create classes, you can see this in the kitchen sink tests where there's a top level LinkType enum: https://github.com/chrisguitarguy/google-api-php-client-services/blob/6f32078f280c4278d8951fadaec618ac9ea67b3f/generator/tests/testdata/golden/php/default/kitchen_sink.golden#L819

and a LinkType enum under SeriesCounters: https://github.com/chrisguitarguy/google-api-php-client-services/blob/6f32078f280c4278d8951fadaec618ac9ea67b3f/generator/tests/testdata/golden/php/default/kitchen_sink.golden#L858

There's not really a way to determine if the schema is actually a reference or not in the generator code and I didn't want to dig too far into that before asking for feedback. If a schema has already been defined, the reference is just resolved directly: https://github.com/chrisguitarguy/google-api-php-client-services/blob/6f32078f280c4278d8951fadaec618ac9ea67b3f/generator/src/googleapis/codegen/schema.py#L200-L207

[bshaffer]

This is amazing work that you've done here! However, this is a fairly old library that's been in maintenance mode more-or-less for 5 years. So adding a new feature like this is not something I personally would want to do unless there's a very good justification.

[chrisguitarguy]

"maintenance mode" but this is still the only PHP SDK that google provides right?

[bshaffer]

We have more resources for our PHP Cloud Client Libraries and PHP Client Libraries for Google Ads, but yes, for all other APIs this is pretty much it (and if it were me, I'd probably just use the Google Auth Library to make HTTP calls...)

But that bit aside, we still need a justification, since this library is quite large and a change like this would generate... I don't even know how many, but... quite a few new files. So let's forget the "maintenance mode" part for now, and tell me why this change is justified.

[chrisguitarguy]

We (PMG) use this library as part of our Alli platform and one thing that we do with the enum stuff (in a fork of our own) is do a bit of validation before we send out API calls. So that's my particular use case: letting non-technical users which values are okay to send by doing some work with reflection on this library. But in general: you have enum data, why not provide it in client libraries?

the stuff here was inspired by the google ads SDK for what that's worth: https://github.com/googleads/google-ads-php/blob/main/src/Google/Ads/GoogleAds/V10/Enums/AccessInvitationStatusEnum/AccessInvitationStatus.php

[devjerry0]

any chance this will be merge in the near future ?

[vishwarajanand]

Hi team, we had an internal discussion per this PR and it seems this PR will generate a lott of new files which might not be in the best from stability perspective, given the repo is in maintenance mode.
So, we are closing this PR for now. In case we do an overhaul of this repo in future, we will surely re-look at it.

[bshaffer]

you have enum data, why not provide it in client libraries?

@chrisguitarguy This is a good point. My main concern is that this library is already timing out in some usecases because of its size, and I am worried adding all the enum data will make this worse. We could do something like add the enum data to the PHPDoc block, like:

class KitchSink
{
    /**
     * Set field "LinkType", possible values are 
     *     - DOWNLOAD
     *     - THUMBNAIL
     *     - PDF
     */
    public function setLinkType($linkType)
    {
    }
}

[chrisguitarguy]

I guess my ask, at least for our specific use case, would be that it's something programmatically parseable.

That could be a "custom" php doc tag or something:

/**
 * @google-oneOf {value, value2, valueN}
 */

It's not gonna mean anything to any parser.

If you wanted to add an array constant or something per property with the values, PHPStan's value-of could be used: https://phpstan.org/writing-php-code/phpdoc-types#key-and-value-types-of-arrays-and-iterables

class Whatever
{
  const PROPERTY_NAME_VALUES = ['value1', 'valueN'];

  /**
   * @var value-of<self::PROPERTY_NAME>
   */
  private $propertyName;
}

Or even just an assert in the setter than can be turned off in prod?

assert(in_array($propertyName), ['value1', 'valueN']));

but that's gonna have less good implications for PHP <7.

My main concern is that this library is already timing out in some usecases because of its size, and I am worried adding all the enum data will make this worse.

This seems pretty strange. It should only be as big as the classes folks are using thanks to autoloading.