Allow orphaned @SWG\Operations, resolvable by their nickname

[dave-newson]

This change will allow for "orphan" @swg\Operation annotations to exist away from the Resource tree.
Once all files are parsed, orphans will be merged on top of elements in the Resource tree which have the same "unique" nickname.

The purpose of this change is to allow Routing-related annotations (resource::resourcePath, Api::path, operation::httpMethod) to exist away from description, parameter and return annotations, for systems where a router file exists separately to controller code. This makes life easier for developers who then can define the Swagger annotated paths within the routes files, and controller-related comments in the controller, making the annotations more DRY and exist where they belong, rather than one or the other.

An example of this notation, the following fixed resource path is defined in the router alongside the routing definition:

    /**
     * @SWG\Resource(
     *      resourcePath="/logs",
     *      @SWG\Api(
     *          path="/",
     *          @SWG\Operation(
     *              nickname="logs.index",
     *              httpMethod="GET"
     *          )
     *      )
     * )
     */

The following orphaned Operation is then defined in the controller:

    /**
     *  @SWG\Operation(
     *      nickname="logs.index",
     *      summary="This function does some magic xyz"
     *  )
     */

The parser recognises the orphan operation annotation and stores it for later. Once all files are parsed, the resource list is inspected for operations with the same nickname, and if found the operation in the resource tree will have it's elements overwritten with those from the orphan operation.

Empty elements are not written, so we don't entirely overwrite the original operation (eg. it's httpMethod).
If no nickname match is found, a notice is raised.

Orphans can overwrite multiple entries if the nicknames match. I thought about raising a notice for this, but it might be someone's intended functionality (multiple routes for one controller action?). As a result, the reunite function is slow because it crawls the entire registry for each orphan.

[bfanger]

Looks very promising, i'll take a more thorough look at it tomorrow, could you write an unittest, so we don't accidentally break this behavior?

PS.
Any ideas for referencing the api without (ab)using the nickname, making the connection more explicit?
Because nicknames will are automaticly generated based on methodname so it could cause accidental matches.

[dave-newson]

I haven't found anything within the Swagger spec that would work besides the nickname, however if you don't mind the idea of adding a custom attribute for this purpose, especially as it's explicitly for compiling the documentation under swagger-php, perhaps we could reinvent this change into AbstractAnnotation?

This way you could detect an orphaned annotation with the pairing attribute and you could track annotations inside the tree which also have a pairing attribute. Crawling that reduced list would take a lot less effort, and you could pair way more annotation types than just operations.. if you don't mind giving people that sort of power.

Another option is giving annotations an optional "name" attribute, and you could have further attributes like "pairto" or "childof", or something like that, to greater expand on the idea, but that might be going too far.

[bfanger]

I think an implementation in AbstractAnnotation would be overkill, I think the logical use-cases are for Apis & Operations.

If we optimize for those use-case we can use resource and api as back references.

/**
 * @SWG\Api(
 *   resource="/pets",
 *   path="/pets/{petId}"
 * )
 */

/**
 * @SWG\Operation(
 *   api="/pets/{petId}",
 *   httpMethod="GET"
 * )
 */

This makes it clear what is going, without adding additional concepts or needing to know for which (nick)name belonged to what Api. etc

An added bonus: it allows the Parser to directly know when an @Api or @operation is meant as orphan, making it easy to show the location of an invalid docblock.

[dave-newson]

It's an improvement, but I think there are still use problems with this approach.
We're now defining the path used by the interface in two places, which was a problem I was originally trying to avoid.
The syntax implies you'd use only the one @operation, which I'd place by my controller code, so the httpMethod that's defined in Operation is now over with the controller code, when it's really related to routing. I'm guessing you could technically still have two @operations with the same path and method, it just doesn't imply that fact.
As you can define multiple operations on a single @Api path, we'd also have to unite operationsonly with matching httpMethod fields, as per the above. This adds an extra complication to the code.

I'd prefer to keep the path definition as part of the DRY approach. I realise if you mistakenly changed the @Api path and forget to change the @operation path you'll probably end up with a non-unitable orphan, and thus a notice, but it's better if you don't have to remember to do that.

Also the parser won't know about the @Api use as an orphan parent, as they could be read in any order and need resolving afterwards. As per the above, there's still cases were you'd want an @operation in an @Api, but still be able to attach orphans. The only way to get around this is to add a label to an @Api (eg. name) that implies it'll be used for orphan resolution, so we know to keep it aside.

More thoughts welcome?

[bfanger]

I've merged your patch into the partials branch, but reimplemented it using AbstractAnnotation (as you suggested) and renamed the more powerful feature to "partials" which allows merging or appending of any annotation-type.

I've used your use-case in the fixtures for the unittest

/**
 * @SWG\Resource(
 *      resourcePath="/logs",
 *      @SWG\Api(
 *          path="/",
 *          @SWG\Operation(
 *              httpMethod="GET",
 *              nickname="logidx",
 *              @SWG\Partial("logs.index")
 *          )
 *      )
 * )
 */

/**
 *  @SWG\Operation(
 *      partial="logs.index",
 *      summary="This function does some magic xyz"
 *  )
 */

What do you think?

[dave-newson]

Looks good! For direct linking between partials that'll definitely solve the problem on my end.

I still think it might be useful to someone if you were able to attach onto a partial as a child, rather than a merge, but we don't have a use case for that right now, so it's not a pressing issue. Maybe a feature to consider?

[bfanger]

If the annotation types don't match the partial is added as a child.
(Or should be, haven't tested that feature yet ;)