Mattermost-server: Centralize ID validation

Created on 25 Feb 2020  路  6Comments  路  Source: mattermost/mattermost-server

ID validation is done using disparate code throughout the Mattermost server code. We would like the validation to be centralized to a single function or method. Any instance of len(foo) != 26 (etc) should be replaced with the centralized function/method.

Note that there is an existing function model.IsValidId which could either be used or be the basis for the solution.


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-22682

AreTechnical Debt Easy Help Wanted PR Exists TecGo

All 6 comments

@mkraft I would like to work on it. Few thoughts/questions brewing in my mind

  • The bigger problem we are trying to solve is of the visibility of the function. The natural place I look for such methods is in validations.go or something on similar lines. However many of the validations methods are in utils which is an antipattern. I propose creating a validations.go in model package and moving the functions which seem apt.
  • I can also replace the use of != 26 with model.IsValidId. I could find 98 matches across 39 files on a case by case basis.

Thoughts?

@RajatVaryani Yes that sounds good as long as you can put the validations there without dependency cycles.

Folks, I won't be able to contribute. Please make it open to the public.

If no one is working on this, I would like to pick this up!

Hi @shibasisp, just wanted to check if you have any questions about the ticket. Please let us know if you need any help

Apologies for the delay. Been stuck up in some personal work. Will try to wrap it up as soon as possible.

Was this page helpful?
0 / 5 - 0 ratings