Plume/Plume

Fork 24

Add some constraint at database level #342

Merged

elegaanz merged 3 commits from db-constraints into master

2018-12-09 17:44:27 +00:00

elegaanz commented

2018-12-08 19:58:56 +00:00

(Migrated from github.com)

Fixes #79 and should fix #201 and #113 as well

The value of ApiToken is unique
A blog/user with a given ID is unique on its instance
An author can't be twice in a blog or post
AP-related URLs/IDs are unique
Instance domains are unique
Like/reshare by an user on a specific post are unique
Media have an unique path
A tag can't be inserted twice in the same post (actually it can, but once as a hashtag, once as a regular tag)

Fixes #79 and should fix #201 and #113 as well - The `value` of `ApiToken` is unique - A blog/user with a given ID is unique on its instance - An author can't be twice in a blog or post - AP-related URLs/IDs are unique - Instance domains are unique - Like/reshare by an user on a specific post are unique - Media have an unique path - A tag can't be inserted twice in the same post (actually it can, but once as a hashtag, once as a regular tag)

trinity-1686a commented

2018-12-08 21:16:35 +00:00

Owner

There is some issue with migrating on an instance that don't currently met the constrain, I'm getting a could not create unique index "blog_unique" because I've some duplicated blogs (I think those are old tests leaks in my case, but as some will have some constraint violated, we should make sure the migration goes smoothly)
For postgresql this can be done with queries such as

DELETE FROM blogs a USING blogs b WHERE a.id > b.id AND a.actor_id=b.actor_id AND a.instance_id=b.instance_id;

run before applying constrains, however this might lead to other data being deleted (things referencing a duplicate instead of the first entry)

There is some issue with migrating on an instance that don't currently met the constrain, I'm getting a `could not create unique index "blog_unique"` because I've some duplicated blogs (I think those are old tests leaks in my case, but as some will have some constraint violated, we should make sure the migration goes smoothly) For postgresql this can be done with queries such as ```SQL DELETE FROM blogs a USING blogs b WHERE a.id > b.id AND a.actor_id=b.actor_id AND a.instance_id=b.instance_id; ``` run before applying constrains, however this might lead to other data being deleted (things referencing a duplicate instead of the first entry)

trinity-1686a reviewed 2018-12-08 21:39:04 +00:00

migrations/postgres/2018-12-08-175515_constraints/up.sql

trinity-1686a commented

2018-12-08 21:39:03 +00:00

Owner

This constrain seems to forbid from having more than one remote medias (it allows for only one null)

codecov[bot] commented

2018-12-09 11:43:40 +00:00

(Migrated from github.com)

Codecov Report

Merging #342 into master will increase coverage by 0.29%.
The diff coverage is 98.9%.

@@            Coverage Diff             @@
##           master     #342      +/-   ##
==========================================
+ Coverage   28.23%   28.53%   +0.29%     
==========================================
  Files          62       62              
  Lines        5592     5607      +15     
==========================================
+ Hits         1579     1600      +21     
+ Misses       4013     4007       -6

Impacted Files	Coverage Δ
plume-models/src/users.rs	`40.17% <100%> (-0.15%)`	⬇️
plume-models/src/blogs.rs	`59.4% <100%> (+4.02%)`	⬆️
plume-models/src/search/mod.rs	`98.16% <100%> (ø)`	⬆️
plume-models/src/medias.rs	`53.95% <80%> (-0.68%)`	⬇️
src/template_utils.rs	`0% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4182e67...160d74d. Read the comment docs.

# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/342?src=pr&el=h1) Report > Merging [#342](https://codecov.io/gh/Plume-org/Plume/pull/342?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/4182e67dbf2deffa34fb5e240ec6fa27e6e8ff4d?src=pr&el=desc) will **increase** coverage by `0.29%`. > The diff coverage is `98.9%`. [![Impacted file tree graph](https://codecov.io/gh/Plume-org/Plume/pull/342/graphs/tree.svg?width=650&token=sHtxmDuZ06&height=150&src=pr)](https://codecov.io/gh/Plume-org/Plume/pull/342?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master #342 +/- ## ========================================== + Coverage 28.23% 28.53% +0.29% ========================================== Files 62 62 Lines 5592 5607 +15 ========================================== + Hits 1579 1600 +21 + Misses 4013 4007 -6 ``` | [Impacted Files](https://codecov.io/gh/Plume-org/Plume/pull/342?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [plume-models/src/users.rs](https://codecov.io/gh/Plume-org/Plume/pull/342/diff?src=pr&el=tree#diff-cGx1bWUtbW9kZWxzL3NyYy91c2Vycy5ycw==) | `40.17% <100%> (-0.15%)` | :arrow_down: | | [plume-models/src/blogs.rs](https://codecov.io/gh/Plume-org/Plume/pull/342/diff?src=pr&el=tree#diff-cGx1bWUtbW9kZWxzL3NyYy9ibG9ncy5ycw==) | `59.4% <100%> (+4.02%)` | :arrow_up: | | [plume-models/src/search/mod.rs](https://codecov.io/gh/Plume-org/Plume/pull/342/diff?src=pr&el=tree#diff-cGx1bWUtbW9kZWxzL3NyYy9zZWFyY2gvbW9kLnJz) | `98.16% <100%> (ø)` | :arrow_up: | | [plume-models/src/medias.rs](https://codecov.io/gh/Plume-org/Plume/pull/342/diff?src=pr&el=tree#diff-cGx1bWUtbW9kZWxzL3NyYy9tZWRpYXMucnM=) | `53.95% <80%> (-0.68%)` | :arrow_down: | | [src/template\_utils.rs](https://codecov.io/gh/Plume-org/Plume/pull/342/diff?src=pr&el=tree#diff-c3JjL3RlbXBsYXRlX3V0aWxzLnJz) | `0% <0%> (ø)` | :arrow_up: | ------ [Continue to review full report at Codecov](https://codecov.io/gh/Plume-org/Plume/pull/342?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/342?src=pr&el=footer). Last update [4182e67...160d74d](https://codecov.io/gh/Plume-org/Plume/pull/342?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

trinity-1686a approved these changes 2018-12-09 12:42:02 +00:00

trinity-1686a left a comment

Owner

works fine on postgresql. I assume you tested sqlite by yourself

migrations/postgres/2018-12-08-175515_constraints/down.sql

trinity-1686a commented

2018-12-09 12:41:16 +00:00

Owner

this constraint won't exist (won't fail because drop if exists, but also useless)

No reviewers

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Plume/Plume#342

No description provided.

Rows
Columns