Nelmioapidocbundle: [JMSModelDescriber] Title and Description missing on virtual properties.

[cyberemissary]

When using VirtualProperty the title and description do not get populated in documentation:

Results in:

A current workaround is to create an actual property which will not be used and define its accessor to be the method used for virtual property like so:

Resulting in:

I have narrowed it down to this section of the code in Nelmio\ApiDocBundle\ModelDescriber\JMSModelDescriber

In case of VirtualPropertyMetadata, the $item->reflection is null so $annotationsReader->updateProperty() (which parses the title/description) is never called.

It should be possible to use method reflection to load this info.

[goetas]

@GuilhemN I would like to start working on this but all the
ModelDescriber/Annotations/SwgAnnotationsReader.php
ModelDescriber/Annotations/AnnotationsReader.php
ModelDescriber/Annotations/PropertyPhpDocReader.php

Always try to extract metadata from ReflectionPropery, but in this case we have ReflectionMethod, so the method signature do not allow to use them for methods but only for properties.

Are you ok if I duplicate the methods to support ReflectionMethod? or you prefer to remove the signatures from the exiting methods?

(I'm not happy with code duplication, but IMO we should duplicate the methods and handle ReflectionMethod independently)

[GuilhemN]

It looks overkill to me to duplicate all these methods, I think removing the signature would be fine (and call the correct doctrine reader method).
Why do you think we should have separate methods to handle ReflectionMethod?

[goetas]

The names are also updateProperty or getPropertyName that gives me the idea of "properties" instead of methods.

And of course inside the classes there will be always some kind of if/else to decide if is property or method reflection... and since will have do be duplicate logic anyway, to avoid cyclomatic complexity with if conditions, was thinking about dedicated methods

[GuilhemN]

I think updateProperty is more about updating a swagger property rather than reading a php one, but theses names may be updated (they're internal) if you think they're not clear enough ;)

About the if/else I think we should create util methods in our AnnotationReader deciding what method to call since the only changes I see needed are these calls to get*Annotation. If I'm correct there will be no duplication and only two if/else added then, right ?

I feel incomfortable with creating new methods: I believe they will create a lot of duplication that won't be worth the absence of if/else, but I'd be glad to be proven wrong if you think it's best :)

[kevinpapst]

Was there any progress on the topic of missing virtual properties in the api docs?
For me that problem is not "just the api docs", I can live with that ... but it goes a little bit further: as pure virtual properties (defined in yaml) do not show up in the docs, they are missing in the swagger json as well.
When now using the swagger code generator, these fields will be missing in the generated code and other consumers are unable to reach those fields (when using the swagger code).

Any idea how to workaround this? Polluting the class with these fields/methods doesn't sound very charming. Can I help on fixing this?

[GuilhemN]

I updated the loaders to support both ReflectionMethod and ReflectionProperty in https://github.com/nelmio/NelmioApiDocBundle/pull/1678 but this is only leveraged by the Symfony serializer's describer for now.

[goetas]

@GuilhemN thanks for this update! Unfortunately I had to pause the extensive work on this library, I hope to get again a project where this feature is needed.

[kevinpapst]

Thank you @GuilhemN for working on that feature!
I assume that does not cover VirtualProperties on class level right now?

[GuilhemN]

This issue should be fixed by https://github.com/nelmio/NelmioApiDocBundle/pull/1682 :)

I assume that does not cover VirtualProperties on class level right now?

This is already covered by using @SWG\Definition at the class level:

/**
 * @SWG\Definition(
 *     required={"id", "user"},
 *     @SWG\Property(property="virtual", ref=@Model(type=JMSUser::class))
 * )
 */
class MyObject { }

Edit: I may have misunderstood your question @kevinpapst, it depends what field you use at the class level, we will support virtual properties with a getter or setter but not more complicated ones like those using expressions, etc.

[kevinpapst]

Sorry for asking like a noob @GuilhemN - I know thats annoying...

What I have at the moment is something like this (with JMSSerializer):

/**
 * @Serializer\VirtualProperty(
 *      "ProjectName",
 *      exp="object.getProject() === null ? null : object.getProject().getName()",
 *      options={
 *          @Serializer\SerializedName("parentTitle"),
 *          @Serializer\Type(name="string"),
 *          @Serializer\Groups({"Activity"})
 *      }
 * )
 */
class Activity {}

Works, but only half-way:

My question was: can I get a title/description for the VirtualProperty parentTitle into the API docs with that setup?

[GuilhemN]

Don't worry, it's a pleasure to answer questions when asked nicely ;) And I know our documentation is lacking information unfortunately...

What you can do is using a complementary class level annotation:

/**
 * @SWG\Definition(
 *     @SWG\Property(property="parentTitle", description="the description", title="the title"))
 * )
 */
class Activity { }

In terms of DX, I suppose we could allow using just @SWG\Property (and remove the nesting with @SWG\Definition) but I'm not sure we could improve this further, parsing the expression looks like a no-go to me and there is no intuitive place where such a description could go imo.

[kevinpapst]

Wow, that was quick.Thanks @GuilhemN ! And I searched and tried for hours without luck ... yeah the documentation issue, we (maintainers of some software) all know that topic 😁

But I run promptly into another issue: your solution works for one property, can I make that working for multiple virtual properties as well?

This don't work:

 * @SWG\Definition(
 *     @SWG\Property(property="project", description="a project id", title="the project")),
 *     @SWG\Property(property="parentTitle", description="the ss description", title="the title"))
 * )

Neither does this:

 * @SWG\Definition(
 *     @SWG\Property(property="project", description="a project id", title="the project"))
 * )
 * @SWG\Definition(
 *     @SWG\Property(property="parentTitle", description="the description", title="the title"))
 * )

The first one that is found is displayed, the other one is silently skipped.
Can I nest multiple @SWG\Property on class level and if so: how?

Sorry for polluting this closed issue, but I figure this is still related to the topic "Title and Description missing on virtual properties"!?

[GuilhemN]

The first solution you tried is the one that is supported :blush:
There is a small typo in it though, there is an extra parenthesis after your @SWG\Property annotations :wink:

Having an exception in this case could help debug, contributions are greatly appreciated in case you'd like to introduce this small but super important change :)

[kevinpapst]

Oh please, things that happen when your child wakes you up after 5 hours sleep ... what a shame 😊

Yes, works as expected after fixing the code LOL Thank you one more time for spotting 👍

I had a quick look and the SwgAnnotationsReader receives just one property from the Doctrine DocParser in that case. It's a stupid typo that Doctrine unfortunately "understands wrong". Reformatted it looks like this:

@SWG\Definition(
     @SWG\Property(property="project", description="a project id", title="the project")
)
,
@SWG\Property(property="parentTitle", description="the description", title="the title")
))

Obviously not what I intended - a case where the human eye sees the error, but the Doctrine parser accepts it as valid.

So the second Property was "ignored" because getClassAnnotation is only called with SwgDefinition::class.
Learned something new about the internals of the bundle.

introduce this small but super important change

Happy to contribute, but I am not sure what you refer to ... did you mean a check for dummies like me?

        if (null !== $this->annotationsReader->getClassAnnotation($reflectionClass, SwgProperty::class)) {
            throw new \Exception('SWG\Property is not supported as class level annotation');
        }

[GuilhemN]

Oh please, things that happen when your child wakes you up after 5 hours sleep ... what a shame 😊

Yes, works as expected after fixing the code LOL Thank you one more time for spotting 👍

Ahah no problem, these things happen 😛

Happy to contribute, but I am not sure what you refer to ... did you mean a check for dummies like me?

I was thinking about something even more general like the following:

if (!empty(array_filter($this->annotationsReader->getClassAnnotations($reflectionClass), function ($v) { return $v instanceof SWG\AbstractAnnotation && !$v instanceof SWG\Definition; }))) { 
    ... 
}

As this is not a well documented feature, I think we should at least guide our users the best we can and indicate wrong usage, annotations, etc, and I believe strict verifications help achieve this goal :)

[kevinpapst]

Can we be sure, that no one has SWG annotations in the class docblock for some other reason?
Just asking because it sounds like a possible BC break.

[GuilhemN]

I can't think of an use case but we can trigger a deprecation just to be sure :)