kitware-resonant / dkc-next

Apache License 2.0
3 stars 0 forks source link

Quota representation and behavior in model layer #41

Closed zachmullen closed 3 years ago

zachmullen commented 4 years ago

The scope of this PR is just to add the database representation of quotas, not to actually add behavior to enforce them.

I went with the existing strategy, and the one that seems to be recommended in Django land, of using a OneToOneField to effectively attach columns to user rows, but using a separate relation. The performance implications, as far as I can guess, are negligible in both directions simply due to how we use quotas. However, there are other tradeoffs that merit consideration:

Pros:

Cons:

FWIW I'm fine with the approach here, I just want us to make an informed decision. We're taking on a little bit of debt in this approach by forcing our app layer to have more consistency checks.

zachmullen commented 4 years ago

PTAL, I've force pushed a new initial commit intended to capture the data modeling and quota resolving behavior. The approach as written is:

  1. Every user, when created, gets a quota associated with their user record. A quota is a tuple of two strictly integer values: amount used, and amount allowed.
  2. Every folder has an owner field, meaning each and every folder in the system is considered to be owned by one user.
  3. Every root folder, when created, gets a quota associated with it. A folder quota record contains a strictly integer valued amount used column, and a nullable integer allowed column. A null allowed column on a folder means that the allowed amount is actually the owning user's allowed amount.
  4. The process of converting a folder from counting against the user's quota to instead having its own quota involves setting a non-null allowed amount as the folder quota.

This approach means that we will need to specially handle when an admin user flips a folder quota from null to non-null, by transferring the used amount from the owning user's pile to the folder's custom allotment. In that case it's just a matter of subtracting from the user's used value, as the folder's own used should already be stored. I haven't written this logic yet -- I plan to put all quota computation and enforcement logic on future PRs, if this approach is approved.

zachmullen commented 4 years ago

I went ahead and stuck rudimentary quota accounting, as well as enforcement, on this branch.

zachmullen commented 4 years ago

This now has all the quota features we need for a MVP. The branch is ready for final review, however we shouldn't merge it until we solve one problem that it introduces.

This branch adds a new constraint: all folders must have an owning user. I like this constraint and want to keep it. However, since we don't currently have user authentication, the process of assigning an owner to a newly created folder doesn't exist. As such, on this branch right now, it's impossible to create folders via the REST API. Two paths forward would be:

  1. Hack together something to make every folder have some arbitrary owner that we grab from the database at create time.
  2. Wait to merge this branch until we have user auth in place, make the folder creation REST endpoint only open to authenticated users, and then attach it there.
zachmullen commented 3 years ago

Good news... I've worked around the lack of authentication in the REST API for now. This is ready to merge!