search

developmentseed / osm-teams

Teams for OpenStreetMap!
https://mapping.team
MIT License
26 stars 5 forks source link

Need to move validation logic into PostgreSQL trigger #158

Open guidorice opened 4 years ago

guidorice commented 4 years ago

removeModerator will probably not work correctly if 2+ requests are interleaved, because the delete and the select are not atomic:

https://github.com/developmentseed/osm-teams/blob/master/app/lib/team.js#L281-L285

I reviewed the PostgreSQL docs, and I think the correct thing to do is move this business logic, e.g. "cannot remove osmId because there must be at least one moderator", out of the model and into a database trigger:

Caused by #141 Relates to #155

^ Same issue comes up in removeOwner

see also: https://karolgalanciak.com/blog/2016/05/06/when-validation-is-not-enough-postgresql-triggers-for-data-integrity/

/cc @kamicut

LanesGood commented 4 years ago

@guidorice is this essential to release, or should we consider creating the PostgreSQL trigger an enhancement?

guidorice commented 4 years ago

@LanesGood Marc and I chatted about this and we agreed it is not essential for the release. It would probably only manifest as a bug under high concurrent loads, which this part of the api is not likely to see.

If implemented, it also needs to be documented carefully to make sure the business logic is understandable in the codebase.

Definitely fine to leave in the backlog.

cc @kamicut