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

Binary UUID Conversion Process #321

Closed
sfount opened this issue Apr 16, 2016 · 4 comments
Closed

Binary UUID Conversion Process #321

sfount opened this issue Apr 16, 2016 · 4 comments
Assignees

Comments

@sfount
Copy link
Contributor

sfount commented Apr 16, 2016

Currently server API helper functions that execute SQL expect a binary UUID (buid) to be passed in as a parameter and this is directly templated into the SQL.

I propose that in general we should minimise the use of the binary format in JavaScript itself, that means that all controllers and methods should assume we dealing with strings (as passed from the client) and the conversion takes place at the last possible point (just before the SQL is executed).

For example:
Invoice Receipt Controller

var Invoices = require('finance/Invoices');
var Patients = require('medical/Patients');
var Enterprises = require('admin/Enterprises');

// Request is sent from the client - I can assume this is a string
var invoiceUuid = req.params.invoiceUuid;

// I can assume any function on the server handling uuids will expect a string
Invoices.details(invoiceUuid)
  //.then...

Invoice Controller

function details(invoiceUuid) { 
var sql = //...

// this is the last point of contact before the SQL query - at this point we can convert to binary 
var buid = db.bid(invoiceUuid);

// db.exec(sql, buid)...
}

Pros:

  • Developer never has to question what format a UUID is in - only the very last method requesting information from the database will ever consider binary
  • Uniform standard for treating UUIDs
  • No manipulation required for working with UUIDs as a string within the JavaScript business logic (never done to my knowledge)

Cons:

  • A long chain of requests (transactions for example) may have to be build using more verbose code to ensure the UUID is in the correct state at the correct time
@sfount sfount added this to the 2.x Process milestone Apr 16, 2016
@jniles
Copy link
Collaborator

jniles commented Apr 16, 2016

This makes a lot of sense - JavaScript doesn't really need to know the uuids are binary until needing to interact with the database. Happy to see this improvement.

sfount added a commit that referenced this issue Apr 16, 2016
This commit bring the receipt core branch up to date with master,
updating all of the outdated references to features that have been
updated.

* Updated invoice receipt example to work with binary uuids
* Resolved conflicts with uuid library
* Removed describe.only from end to end tests

This commit also implements the suggested binary UUID conversion
structure described in
[#321](#321). This
ensures that all controllers using UUIDs to make internal requests can
assume they are working with strings. (Moves conversations to the SQL
query step).
sfount added a commit that referenced this issue Apr 16, 2016
This commit bring the receipt core branch up to date with master,
updating all of the outdated references to features that have been
updated.

* Updated invoice receipt example to work with binary uuids
* Resolved conflicts with uuid library
* Removed describe.only from end to end tests

This commit also implements the suggested binary UUID conversion
structure described in
[#321](#321). This
ensures that all controllers using UUIDs to make internal requests can
assume they are working with strings. (Moves conversations to the SQL
query step).
@jniles
Copy link
Collaborator

jniles commented Apr 27, 2016

@IMA-WorldHealth/local-contributors

What would you think of a helper conversion function that converts uuids to binary if they exist, based on an array of keys? For example:

let data = req.body;

// do some logic

// convert the 'uuid' and 'debtor_uuid' properties to binary using db.bid()
data = db.convert(data, ['uuid', 'debtor_uuid']);

// now I can db.exec('INSERT...') or db.exec('UPDATE ...') easily.

It might help clean up the code a bit.

@sfount
Copy link
Contributor Author

sfount commented Apr 27, 2016

This could provide useful functionality.

I would still suggest that this is only done directly before reading or writing to the database - not immediately as the body is received.

The reason for this is passing req.body around with strings allows the controllers to easily access and pass values to other required controllers without considering if it a string or binary.

@jniles
Copy link
Collaborator

jniles commented Apr 27, 2016

@sfount, I agree. Our logs with also be more readable the longer we avoid binary data.

I'll look into implementing this in a PR soon.

jniles added a commit to jniles/bhima that referenced this issue Apr 29, 2016
This commit implements `db.convert()` to convert hexadecimal strings on
objects into binary representation for insertion into the database.  It
is demonstrated on both the patient groups controller and enterprises
controller.

It is used as described here: Third-Culture-Software#321 (comment)
```js
let data = req.body;

// do some logic

// convert the 'uuid' and 'debtor_uuid' properties to binary using
// db.bid()
data = db.convert(data, ['uuid', 'debtor_uuid']);

// now I can db.exec('INSERT...') or db.exec('UPDATE ...') easily.
It might help clean up the code a bit.
```
jniles added a commit to jniles/bhima that referenced this issue Apr 29, 2016
This commit implements `db.convert()` to convert hexadecimal strings on
objects into binary representation for insertion into the database.  It
is demonstrated on both the patient groups controller and enterprises
controller.

It is used as described here: Third-Culture-Software#321 (comment)
```js
let data = req.body;

// do some logic

// convert the 'uuid' and 'debtor_uuid' properties to binary using
// db.bid()
data = db.convert(data, ['uuid', 'debtor_uuid']);

// now I can db.exec('INSERT...') or db.exec('UPDATE ...') easily.
It might help clean up the code a bit.
```
jniles added a commit to jniles/bhima that referenced this issue May 2, 2016
This commit implements `db.convert()` to convert hexadecimal strings on
objects into binary representation for insertion into the database.  It
is demonstrated on both the patient groups controller and enterprises
controller.

It is used as described here: Third-Culture-Software#321 (comment)
```js
let data = req.body;

// do some logic

// convert the 'uuid' and 'debtor_uuid' properties to binary using
// db.bid()
data = db.convert(data, ['uuid', 'debtor_uuid']);

// now I can db.exec('INSERT...') or db.exec('UPDATE ...') easily.
It might help clean up the code a bit.
```
jniles added a commit to jniles/bhima that referenced this issue May 2, 2016
This commit implements `db.convert()` to convert hexadecimal strings on
objects into binary representation for insertion into the database.  It
is demonstrated on both the patient groups controller and enterprises
controller.

It is used as described here: Third-Culture-Software#321 (comment)
```js
let data = req.body;

// do some logic

// convert the 'uuid' and 'debtor_uuid' properties to binary using
// db.bid()
data = db.convert(data, ['uuid', 'debtor_uuid']);

// now I can db.exec('INSERT...') or db.exec('UPDATE ...') easily.
It might help clean up the code a bit.
```
jniles added a commit to jniles/bhima that referenced this issue May 3, 2016
This commit implements `db.convert()` to convert hexadecimal strings on
objects into binary representation for insertion into the database.  It
is demonstrated on both the patient groups controller and enterprises
controller.

It is used as described here: Third-Culture-Software#321 (comment)
```js
let data = req.body;

// do some logic

// convert the 'uuid' and 'debtor_uuid' properties to binary using
// db.bid()
data = db.convert(data, ['uuid', 'debtor_uuid']);

// now I can db.exec('INSERT...') or db.exec('UPDATE ...') easily.
It might help clean up the code a bit.
```
sfount pushed a commit that referenced this issue May 3, 2016
This commit implements `db.convert()` to convert hexadecimal strings on
objects into binary representation for insertion into the database.  It
is demonstrated on both the patient groups controller and enterprises
controller.

It is used as described here: #321 (comment)
```js
let data = req.body;

// do some logic

// convert the 'uuid' and 'debtor_uuid' properties to binary using
// db.bid()
data = db.convert(data, ['uuid', 'debtor_uuid']);

// now I can db.exec('INSERT...') or db.exec('UPDATE ...') easily.
It might help clean up the code a bit.
```
@jniles jniles self-assigned this May 11, 2016
jniles referenced this issue in jniles/bhima May 25, 2016
This commit removes all custom convert() methods from server
controllers, preferring to use db.convert() to change uuids from hex
into binary values before database insertion.

Closes #321.
jniles referenced this issue in jniles/bhima May 27, 2016
This commit removes all custom convert() methods from server
controllers, preferring to use db.convert() to change uuids from hex
into binary values before database insertion.

Closes #321.
jniles referenced this issue in jniles/bhima May 27, 2016
This commit removes all custom convert() methods from server
controllers, preferring to use db.convert() to change uuids from hex
into binary values before database insertion.

Closes #321.
jniles referenced this issue in jniles/bhima May 27, 2016
This commit removes all custom convert() methods from server
controllers, preferring to use db.convert() to change uuids from hex
into binary values before database insertion.

Closes #321.
jniles referenced this issue in jniles/bhima May 30, 2016
This commit removes all custom convert() methods from server
controllers, preferring to use db.convert() to change uuids from hex
into binary values before database insertion.

Closes #321.
jniles referenced this issue in jniles/bhima May 30, 2016
This commit removes all custom convert() methods from server
controllers, preferring to use db.convert() to change uuids from hex
into binary values before database insertion.

Closes #321.
bors bot added a commit that referenced this issue Jan 8, 2019
3530: Update snyk to the latest version 🚀 r=jniles a=greenkeeper[bot]


## The dependency [snyk](https://github.com/snyk/snyk) was updated from `1.120.1` to `1.121.0`.
This version is **not covered** by your **current version range**.

If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update.

---

<details>
<summary>Release Notes for v1.121.0</summary>

<h1><a href="https://urls.greenkeeper.io/snyk/snyk/compare/v1.120.1...v1.121.0">1.121.0</a> (2019-01-08)</h1>
<h3>Features</h3>
<ul>
<li>Split dockerfile / base image vulns (<a href="https://urls.greenkeeper.io/snyk/snyk/commit/b2f530d">b2f530d</a>)</li>
</ul>
</details>

<details>
<summary>Commits</summary>
<p>The new version differs by 4 commits.</p>
<ul>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/ecac358cc1da7cee3715e709cfa368838c598af6"><code>ecac358</code></a> <code>Merge pull request #313 from snyk/feat/docker-split-user-base-vulns</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/b2f530db922c85eed0de6139512e6eb83cb174fa"><code>b2f530d</code></a> <code>feat: Split dockerfile / base image vulns</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/5f6083bb2f4dd80ecd7a3f4f7c8a16aa00f7eb58"><code>5f6083b</code></a> <code>Merge pull request #321 from snyk/chore/disable-lockfile-creation</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/95be92c862655fdda539056893336c4eb05582e3"><code>95be92c</code></a> <code>chore: Disable lockfile creation</code></li>
</ul>
<p>See the <a href="https://urls.greenkeeper.io/snyk/snyk/compare/5938f576c782762d1cbaa9cd4d10afa7dde3b91b...ecac358cc1da7cee3715e709cfa368838c598af6">full diff</a></p>
</details>

<details>
  <summary>FAQ and help</summary>

  There is a collection of [frequently asked questions](https://greenkeeper.io/faq.html). If those don’t help, you can always [ask the humans behind Greenkeeper](https://github.com/greenkeeperio/greenkeeper/issues/new).
</details>

---


Your [Greenkeeper](https://greenkeeper.io) bot 🌴



Co-authored-by: greenkeeper[bot] <greenkeeper[bot]@users.noreply.github.com>
bors bot added a commit that referenced this issue Jan 9, 2019
3530: Update snyk to the latest version 🚀 r=jniles a=greenkeeper[bot]


## The dependency [snyk](https://github.com/snyk/snyk) was updated from `1.120.1` to `1.121.0`.
This version is **not covered** by your **current version range**.

If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update.

---

<details>
<summary>Release Notes for v1.121.0</summary>

<h1><a href="https://urls.greenkeeper.io/snyk/snyk/compare/v1.120.1...v1.121.0">1.121.0</a> (2019-01-08)</h1>
<h3>Features</h3>
<ul>
<li>Split dockerfile / base image vulns (<a href="https://urls.greenkeeper.io/snyk/snyk/commit/b2f530d">b2f530d</a>)</li>
</ul>
</details>

<details>
<summary>Commits</summary>
<p>The new version differs by 4 commits.</p>
<ul>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/ecac358cc1da7cee3715e709cfa368838c598af6"><code>ecac358</code></a> <code>Merge pull request #313 from snyk/feat/docker-split-user-base-vulns</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/b2f530db922c85eed0de6139512e6eb83cb174fa"><code>b2f530d</code></a> <code>feat: Split dockerfile / base image vulns</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/5f6083bb2f4dd80ecd7a3f4f7c8a16aa00f7eb58"><code>5f6083b</code></a> <code>Merge pull request #321 from snyk/chore/disable-lockfile-creation</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/95be92c862655fdda539056893336c4eb05582e3"><code>95be92c</code></a> <code>chore: Disable lockfile creation</code></li>
</ul>
<p>See the <a href="https://urls.greenkeeper.io/snyk/snyk/compare/5938f576c782762d1cbaa9cd4d10afa7dde3b91b...ecac358cc1da7cee3715e709cfa368838c598af6">full diff</a></p>
</details>

<details>
  <summary>FAQ and help</summary>

  There is a collection of [frequently asked questions](https://greenkeeper.io/faq.html). If those don’t help, you can always [ask the humans behind Greenkeeper](https://github.com/greenkeeperio/greenkeeper/issues/new).
</details>

---


Your [Greenkeeper](https://greenkeeper.io) bot 🌴



Co-authored-by: greenkeeper[bot] <greenkeeper[bot]@users.noreply.github.com>
bors bot added a commit that referenced this issue Jan 10, 2019
3530: Update snyk to the latest version 🚀 r=jniles a=greenkeeper[bot]


## The dependency [snyk](https://github.com/snyk/snyk) was updated from `1.120.1` to `1.121.0`.
This version is **not covered** by your **current version range**.

If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update.

---

<details>
<summary>Release Notes for v1.121.0</summary>

<h1><a href="https://urls.greenkeeper.io/snyk/snyk/compare/v1.120.1...v1.121.0">1.121.0</a> (2019-01-08)</h1>
<h3>Features</h3>
<ul>
<li>Split dockerfile / base image vulns (<a href="https://urls.greenkeeper.io/snyk/snyk/commit/b2f530d">b2f530d</a>)</li>
</ul>
</details>

<details>
<summary>Commits</summary>
<p>The new version differs by 4 commits.</p>
<ul>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/ecac358cc1da7cee3715e709cfa368838c598af6"><code>ecac358</code></a> <code>Merge pull request #313 from snyk/feat/docker-split-user-base-vulns</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/b2f530db922c85eed0de6139512e6eb83cb174fa"><code>b2f530d</code></a> <code>feat: Split dockerfile / base image vulns</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/5f6083bb2f4dd80ecd7a3f4f7c8a16aa00f7eb58"><code>5f6083b</code></a> <code>Merge pull request #321 from snyk/chore/disable-lockfile-creation</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/95be92c862655fdda539056893336c4eb05582e3"><code>95be92c</code></a> <code>chore: Disable lockfile creation</code></li>
</ul>
<p>See the <a href="https://urls.greenkeeper.io/snyk/snyk/compare/5938f576c782762d1cbaa9cd4d10afa7dde3b91b...ecac358cc1da7cee3715e709cfa368838c598af6">full diff</a></p>
</details>

<details>
  <summary>FAQ and help</summary>

  There is a collection of [frequently asked questions](https://greenkeeper.io/faq.html). If those don’t help, you can always [ask the humans behind Greenkeeper](https://github.com/greenkeeperio/greenkeeper/issues/new).
</details>

---


Your [Greenkeeper](https://greenkeeper.io) bot 🌴



3537: fix(journal): preserve trans_id_reference_number r=jniles a=jniles

We now remember the trans_id_reference_number when we create new lines in a transaction.  This means we no longer error when trying to add new lines to a transaction.

Note that we still need to add a test.

Partially addresses #3536.

Co-authored-by: greenkeeper[bot] <greenkeeper[bot]@users.noreply.github.com>
Co-authored-by: Jonathan Niles <[email protected]>
bors bot added a commit that referenced this issue Jan 10, 2019
3530: Update snyk to the latest version 🚀 r=jniles a=greenkeeper[bot]


## The dependency [snyk](https://github.com/snyk/snyk) was updated from `1.120.1` to `1.121.0`.
This version is **not covered** by your **current version range**.

If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update.

---

<details>
<summary>Release Notes for v1.121.0</summary>

<h1><a href="https://urls.greenkeeper.io/snyk/snyk/compare/v1.120.1...v1.121.0">1.121.0</a> (2019-01-08)</h1>
<h3>Features</h3>
<ul>
<li>Split dockerfile / base image vulns (<a href="https://urls.greenkeeper.io/snyk/snyk/commit/b2f530d">b2f530d</a>)</li>
</ul>
</details>

<details>
<summary>Commits</summary>
<p>The new version differs by 4 commits.</p>
<ul>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/ecac358cc1da7cee3715e709cfa368838c598af6"><code>ecac358</code></a> <code>Merge pull request #313 from snyk/feat/docker-split-user-base-vulns</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/b2f530db922c85eed0de6139512e6eb83cb174fa"><code>b2f530d</code></a> <code>feat: Split dockerfile / base image vulns</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/5f6083bb2f4dd80ecd7a3f4f7c8a16aa00f7eb58"><code>5f6083b</code></a> <code>Merge pull request #321 from snyk/chore/disable-lockfile-creation</code></li>
<li><a href="https://urls.greenkeeper.io/snyk/snyk/commit/95be92c862655fdda539056893336c4eb05582e3"><code>95be92c</code></a> <code>chore: Disable lockfile creation</code></li>
</ul>
<p>See the <a href="https://urls.greenkeeper.io/snyk/snyk/compare/5938f576c782762d1cbaa9cd4d10afa7dde3b91b...ecac358cc1da7cee3715e709cfa368838c598af6">full diff</a></p>
</details>

<details>
  <summary>FAQ and help</summary>

  There is a collection of [frequently asked questions](https://greenkeeper.io/faq.html). If those don’t help, you can always [ask the humans behind Greenkeeper](https://github.com/greenkeeperio/greenkeeper/issues/new).
</details>

---


Your [Greenkeeper](https://greenkeeper.io) bot 🌴



Co-authored-by: greenkeeper[bot] <greenkeeper[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants