Mattermost-server: Programmatically check plugin helpers minimum server version comment

[lieut-data]

In plugin/api.go, we document the minimum required server version to use an API, e.g.:

// UpdateUserActive deactivates or reactivates an user.
//
// Minimum server version: 5.8
UpdateUserActive(userId string, active bool) *model.AppError

This means that the API was first added to Mattermost 5.8, and simply won't be available in earlier versions. Methods without an explicit comment are implicitly supported in Mattermost 5.2 and later, though this is being separately improved as part of https://mattermost.atlassian.net/browse/MM-17889 .

In plugin/helpers.go, we do the same thing:

// KVSetWithExpiryJSON stores a key-value pair with an expiry time, unique per plugin, marshalling the given value as a JSON string.
//
// Minimum server version: 5.6
KVSetWithExpiryJSON(key string, value interface{}, expireInSeconds int64) error

The helpers API is not exposed in a running server – the helpers may be compiled into the plugin, and internally use the plugin API directly. So the "minimum server version" comments on the helpers are in fact computable as the latest minimum server version on any API call made by the helpers.

Implement a reflective checker that examines the helper methods exposed on the Helpers interface, and their corresponding implementation in HelpersImpl, and determines if the commented minimum server version is correct based on what API methods the helpers call through.

Note that there may sometimes be cases where behaviour was "fixed" in a newer version, and the helper intentionally has a higher minimum. Thus, the tool should not fail if the minimum server version is higher than it need be: just emit a warning instead. But it's absolutely an error for a helper marked for vN to be relying on an API method that was only implemented in, for example, vN-1.

Build this into a plugin/checker package, and run the tool as part of the overall server build to ensure the helpers are always accurately commented.

Note that reflection in general is a tricky subject, and this issue should probably be tagged as 4: Very Hard. It is not recommended for anyone new to Go or who doesn’t have a somewhat working knowledge of Go reflection internals. Do not implement this using grep or similar strategies.

---

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

JIRA: https://mattermost.atlassian.net/browse/MM-17888

[lanre-ade]

Hi @lieut-data, I'd like to tackle this ticket

[pbitty]

There might be a mixup here. I referenced this issue from another PR, but that PR was not for this issue. From what I can tell this issue doesn't have a corresponding PR yet.

[lieut-data]

Thanks for the clarification, @pbitty -- removed the PR Exists label as such.

[hanzei]

Making this one available for the public again due to inactivity.

[pbitty]

Hi @hanzei , I am happy to work on this one. It would be a good continuation from #11910, which I worked on.

[lieut-data]

🎉- I was hoping you would end up continuing your efforts here, @pbitty :)

[pbitty]

Great! I should be able to get started this weekend.

[pbitty]

I have started making progress on this. Not quite ready for a PR yet, but I have been able to:

• extract the minimum version declared in each API and Helpers methods
• enumerate receiver methods of HelpersImpl
• walk through the AST Nodes of each HelpersImpl methods

Now I am working on finding all the method calls to API inside the HelpersImpl methods.

[lieut-data]

Thanks again, @pbitty!

[pbitty]

No prob!