Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make factory name mandatory #4372

Closed
voievodin opened this issue Mar 9, 2017 · 1 comment
Closed

Make factory name mandatory #4372

voievodin opened this issue Mar 9, 2017 · 1 comment
Labels
kind/bug Outline of a bug - must adhere to the bug report template. status/open-for-dev An issue has had its specification reviewed and confirmed. Waiting for an engineer to take it.

Comments

@voievodin
Copy link
Contributor

voievodin commented Mar 9, 2017

The problem

For each factory e.g.

{
  "v": "4.0",
  "name": "my-factory",
  "id": "i0rx6mlihjaayugt",
  "creator": {
    "name": "username",
    "created": 1465332525959,
    "userId": "user1234567"
  },
  "workspace": { }
}

the combination of factory.name + factory.creator.userId must be unique, so the factory
may be accepted as /<namespace>/<factory-name> where the namespace is the name of the user who is the creator of the factory.
Those factories which don't have name, cannot be accepted in the format described above.

Right now multiple factories with the same name may exist, as the unique constraint is missing.
In this case the first factory returned from factory service will be used.

The actual problem here is that the unique constraint like this requires partial unique index, which is not a common feature of database systems. It could look like this:

CREATE UNIQUE INDEX idx ON factory (user_id, name) WHERE (user_id IS NOT NULL)

While it works for PostgreSQL it doesn't for H2.

Proposed solution

Make factory name mandatory on database level + add a unique constraint on (user_id, name).

Each time factory service receives a factory without name it must set its name to some value e.g. factory id or generate a new factory name, so in this way clients are still not required to send factory name so everything works like before

For those factories in database that don't have name create a migration script that sets names equal to factory ids(or the values generated by the same principle as the values set by service) e.g:

factory_id factory_name user_id
factory123 null user123
factory234 null user123

⬇️ becomes ⬇️

factory_id factory_name user_id
factory123 factory123 user123
factory234 factory234 user123

For those factories in database that have duplicated name + user_id create a migration script that
leaves the very first factory with such a name as it is and modifies all the other factories in a way defined by the script from the previous statement. So the factories with such a name are accepted as they were(the first one) e.g:

factory_id factory_name user_id
factory123 example user123
factory234 example user123

⬇️ becomes ⬇️

factory_id factory_name user_id
factory123 example user123
factory234 factory234 user123
@voievodin voievodin added the kind/bug Outline of a bug - must adhere to the bug report template. label Mar 9, 2017
@voievodin
Copy link
Contributor Author

Check out factory_migration branch, it provides factory schema

@skabashnyuk skabashnyuk added sprint/next team/platform status/open-for-dev An issue has had its specification reviewed and confirmed. Waiting for an engineer to take it. labels Mar 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Outline of a bug - must adhere to the bug report template. status/open-for-dev An issue has had its specification reviewed and confirmed. Waiting for an engineer to take it.
Projects
None yet
Development

No branches or pull requests

3 participants