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

lb3 - Relations on uuid #4287

Closed
fasenderos opened this issue Nov 29, 2019 · 15 comments
Closed

lb3 - Relations on uuid #4287

fasenderos opened this issue Nov 29, 2019 · 15 comments

Comments

@fasenderos
Copy link

Steps to reproduce

When the foreignKey of the relations point to ID everything works as expected, while if I try to point the foreignKey to UUID, the UUID is parsed as integer (NaN) and the relation not work.

To reproduce create 2 models:

Customer

{
  "name": "customer",
  "base": "PersistedModel",
  "idInjection": true,
  "options": {
    "validateUpsert": true
  },
  "properties": {
    "id": {
      "type": "number"
    },
    "uuid": {
      "type": "string",
      "defaultFn": "uuidv4"
    },
    "name": {
      "type": "string"
    }
  },
  "validations": [],
  "relations": {
    "addresses": {
      "model": "address",
      "type": "hasMany",
      "foreignKey": "customerId"
    }
  },
  "acls": [],
  "methods": {}
}

Address

{
  "name": "address",
  "base": "PersistedModel",
  "idInjection": true,
  "options": {
    "validateUpsert": true
  },
  "properties": {
    "id": {
      "type": "number"
    },
    "uuid": {
      "type": "string",
      "defaultFn": "uuidv4"
    },
    "customerId": {
      "type": "number",
      "mysql": {
        "columnName": "customer_id"
      }
    },
    "customerUuid": {
      "type": "string",
      "mysql": {
        "columnName": "customer_uuid"
      }
    },
    "street": {
      "type": "string"
    }
  },
  "validations": [],
  "relations": {
    "customer": {
      "model": "customer",
      "type": "belongsTo",
      "foreignKey": "customerId"
    }
  },
  "acls": [],
  "methods": {}
}

Get customer with address related by ID:

const customer = await app.models.Customer.findById(1, {  include: { relation: 'addresses' } });
console.log(customer);

work

Now in Address replace "foreignKey": "customerId" with "foreignKey": "customerUuid" and you get customerUuid as NaN:
nan

If you do the same thing for Customer model by replacing the "foreignKey": "customerId" with "foreignKey": "customerUuid", you get an empty addresses list
empty

Expected Behavior

The ability to use UUID on to relate model

Additional information

Node: v10.14.1
Npm: v6.4.1
Database: 10.1.30-MariaDB

lb --version
5.0.3 ([email protected] [email protected])

"dependencies": {
    "compression": "^1.0.3",
    "cors": "^2.5.2",
    "helmet": "^3.10.0",
    "loopback": "^3.22.0",
    "loopback-boot": "^2.6.5",
    "loopback-component-explorer": "^6.2.0",
    "loopback-connector-mysql": "^5.4.2",
    "serve-favicon": "^2.0.1",
    "strong-error-handler": "^3.0.0"
  }
@fasenderos
Copy link
Author

Maybe here is same issue on LB4 loopbackio/loopback-next#3602 that seems to be solved

@bajtos
Copy link
Member

bajtos commented Nov 29, 2019

Thank you for reporting the issue. Since it's related to LoopBack 3.x, I am going to move it to strongloop/loopback repository.

@bajtos bajtos transferred this issue from loopbackio/loopback-next Nov 29, 2019
@bajtos bajtos added the bug label Nov 29, 2019
@bajtos
Copy link
Member

bajtos commented Nov 29, 2019

Maybe here is same issue on LB4 loopbackio/loopback-next#3602 that seems to be solved

This looks like a reasonable assumption to me. Can you try to apply loopbackio/loopback-datasource-juggler#1783 to the juggler module installed in your dependencies and then add useDefaultIdType: false to uuid property definition?

@fasenderos
Copy link
Author

Thank you for your reply.
I replace in loopback-datasource-juggler/lib/datasource.js on line 711
if (idProp && idProp.generated && this.connector.getDefaultIdType)
with
if (idProp && idProp.generated && idProp.useDefaultIdType !== false && this.connector.getDefaultIdType)

Then edited Customer and Address models as below:

### Customer and Address
"uuid": {
   "type": "string",
   "useDefaultIdType": false,
   "defaultFn": "uuidv4"
}

### Address
"customerUuid": {
    "type": "string",
    "useDefaultIdType": false,
    "mysql": {
      "columnName": "customer_uuid"
    }
}

But I get same results, uuid NaN.

@bajtos
Copy link
Member

bajtos commented Nov 29, 2019

Hmm, then the problem may be somewhere else.

@fasenderos
Copy link
Author

fasenderos commented Nov 29, 2019

The juggler always look at the id prop, so adding "id": true to uuid property make the relation work again:

"uuid": {
   "type": "string",
   "id": true,
   "useDefaultIdType": false,
   "defaultFn": "uuidv4"
}

UPDATE: works even without useDefaultIdType

"uuid": {
   "type": "string",
   "id": true,
   "defaultFn": "uuidv4"
}

@fasenderos
Copy link
Author

fasenderos commented Dec 2, 2019

Although the model relations now seem to work fine, most of the methods don't.

For ex Customer.create() returns an object without the ID prop but with the auto incremented id in the UUID prop, like this:

"uuid": 100, -> The auto incremented ID
"name": "John Doe"

While in the db the entry is:

"id": 100
"uuid": "281d25ba-1685-463f-812a-0f1c2b77bbd9",
"name": "John Doe"

@agnes512
Copy link
Contributor

agnes512 commented Jan 2, 2020

@fasenderos I think LB3 takes the id property as the source key by default. That's why when set the uuid property to id: true the relation works. I am not sure if lb3 can customize the source key. Can you try to set the primary key of relations as:

Customer

... customer mode def
  "properties": {
    "id": {
      "id": true,   // model primary key is still id 
      "generated": true,
      "type": "number"
    },
    "uuid": {
      "type": "string",
      "defaultFn": "uuidv4"
    },
   }..
.....
  "relations": {
    "addresses": {
      "model": "address",
      "type": "hasMany",
      // edit: should be "customerUuid"
      "foreignKey": "customerId",
      "primaryKey": "uuid" . // customized primary key
    }
  },

Address

... address mode def
  "relations": {
    "addresses": {
      "model": "customer",
      "type": "belongsTo",
      // edit: should be "customerUuid"
      "foreignKey": "customerId",
      "primaryKey": "uuid" .   // customized primary key
    }
  },

I am also curious about the result of Customer.create() . I have no clue why it returns the wrong uuid and ignores id. Do you mind posting your Customer model definition here? thanks.

@fasenderos
Copy link
Author

fasenderos commented Jan 3, 2020

I am also curious about the result of Customer.create() . I have no clue why it returns the wrong uuid and ignores id. Do you mind posting your Customer model definition here? thanks.

@agnes512 first I post my model def without your suggestions

Customer

{
  "name": "customer",
  "base": "PersistedModel",
  "idInjection": false,
  "options": {
    "validateUpsert": true
  },
  "properties": {
    "id": {
      "type": "number"
    },
    "uuid": {
      "id": true,
      "type": "string",
      "defaultFn": "uuidv4"
    },
    "name": {
      "type": "string"
    }
  },
  "validations": [],
  "relations": {
    "addresses": {
      "model": "address",
      "type": "hasMany",
      "foreignKey": "customerUuid"
    }
  },
  "acls": [],
  "methods": {}
}

Address

{
  "name": "address",
  "base": "PersistedModel",
  "idInjection": false,
  "options": {
    "validateUpsert": true
  },
  "properties": {
    "id": {
      "type": "number"      
    },
    "uuid": {
      "id": true,
      "type": "string",      
      "defaultFn": "uuidv4"
    },
    "customerId": {
      "type": "number",
      "mysql": {
        "columnName": "customer_id"
      }
    },
    "customerUuid": {
      "type": "string",      
      "mysql": {
        "columnName": "customer_uuid"
      }
    },
    "street": {
      "type": "string"
    }
  },
  "validations": [],
  "relations": {
    "customer": {
      "model": "customer",
      "type": "belongsTo",
      "foreignKey": "customerUuid"
    }
  },
  "acls": [],
  "methods": {}
}

Then I create a boot script /boot/test.js

'use strict';

module.exports = async function (app) {
    let Customer = app.models.Customer;
    let Address = app.models.Address;

    // Create Customer
    let createdCustomer = await Customer.create({ name: 'John Doe' });
    console.log("Customer.create() ", createdCustomer);    

    // Retrieve customer by ID to get the UUID of the customer
    let customer = await Customer.findOne({
        where: { id: createdCustomer.uuid } // <------ UUID instead of ID
    });
    console.log("\nCustomer.findOne()", customer);

    // Create address with customer reference
    let createdAddress = await Address.create({customerUuid: customer.uuid, customerId: customer.id, street: 'Route 66'});
    console.log("\nAddress.create() ", createdAddress);

    // Retrieve address by ID to get the UUID of the address
    let address = await Address.findOne({
        where: { id: createdAddress.uuid } // <------ UUID instead of ID
    });
    console.log("\nAddress.findOne()", address);
    
    /*** Now check if the customer and addresses relations by uuid works ***/
    // Get customer with addresses relation
    let getCustomer = await Customer.findOne({
        where: { uuid: customer.uuid },
        include: { relation: 'addresses' }
    });
    console.log("\nCustomer with addresses relation ", getCustomer);

    // Get address with customer relation
    let getAddress = await Address.findOne({
        where: { uuid: address.uuid },
        include: { relation: 'customer' }
    });    
    
    console.log("\nAddress with customer relation ", getAddress);
}

These are the results
Annotazione 2020-01-03 104553

@fasenderos
Copy link
Author

fasenderos commented Jan 3, 2020

@fasenderos I think LB3 takes the id property as the source key by default. That's why when set the uuid property to id: true the relation works. I am not sure if lb3 can customize the source key. Can you try to set the primary key of relations as:

@agnes512 with your suggestions the create() method works right, but the relations don't. Here is my model def with your suggesstions:

Customer

{
  "name": "customer",
  "base": "PersistedModel",
  "idInjection": false,
  "options": {
    "validateUpsert": true
  },
  "properties": {
    "id": {
      "id": true,
      "generated": true,
      "type": "number"
    },
    "uuid": {      
      "type": "string",
      "defaultFn": "uuidv4"
    },
    "name": {
      "type": "string"
    }
  },
  "validations": [],
  "relations": {
    "addresses": {
      "model": "address",
      "type": "hasMany",
      "foreignKey": "customerId",
      "primaryKey": "uuid"
    }
  },
  "acls": [],
  "methods": {}
}

Address

{
  "name": "address",
  "base": "PersistedModel",
  "idInjection": false,
  "options": {
    "validateUpsert": true
  },
  "properties": {
    "id": {
      "id": true,
      "generated": true,
      "type": "number"      
    },
    "uuid": {      
      "type": "string",
      "defaultFn": "uuidv4"
    },
    "customerId": {
      "type": "number",
      "mysql": {
        "columnName": "customer_id"
      }
    },
    "customerUuid": {
      "type": "string",      
      "mysql": {
        "columnName": "customer_uuid"
      }
    },
    "street": {
      "type": "string"
    }
  },
  "validations": [],
  "relations": {
    "customer": {
      "model": "customer",
      "type": "belongsTo",
      "foreignKey": "customerId",
      "primaryKey": "uuid"
    }
  },
  "acls": [],
  "methods": {}
}

Edited boot script /boot/test.js

'use strict';

module.exports = async function (app) {
    let Customer = app.models.Customer;
    let Address = app.models.Address;

    // Create Customer
    let createdCustomer = await Customer.create({ name: 'John Doe' });
    console.log("Customer.create() ", createdCustomer);    

    /** NO MORE NEED to retrieve the customer because now we get the UUID **/

    // Create address with customer reference
    let createdAddress = await Address.create({customerUuid: createdCustomer.uuid, customerId: createdCustomer.id, street: 'Route 66'});
    console.log("\nAddress.create() ", createdAddress);

    /** NO MORE NEED to retrieve the address because now we get the UUID **/
    
    /*** Now check if customer and addresses relations by uuid works ***/
    // Get customer with addresses relation
    let getCustomer = await Customer.findOne({
        where: { uuid: createdCustomer.uuid },
        include: { relation: 'addresses' }
    });
    console.log("\nCustomer with addresses relation ", getCustomer);

    // Get address with customer relation
    let getAddress = await Address.findOne({
        where: { uuid: createdAddress.uuid },
        include: { relation: 'customer' }
    });    
    
    console.log("\nAddress with customer relation ", getAddress);
}

These are the results
Annotazione 2

@agnes512
Copy link
Contributor

agnes512 commented Jan 3, 2020

@fasenderos sorry I made a mistake! Both relation definitions in my suggestion should be :

      "foreignKey": "customerUuid", // not customerId
      "primaryKey": "uuid"

Could you try it one more time? thanks!

uuid should be working fine when it's the id property. Just need to check if it's possible to customize the primary key in LB3 so that we can have both int type id and string type uuid.

@fasenderos
Copy link
Author

@fasenderos sorry I made a mistake! Both relation definitions in my suggestion should be :

      "foreignKey": "customerUuid", // not customerId
      "primaryKey": "uuid"

Could you try it one more time? thanks!

uuid should be working fine when it's the id property. Just need to check if it's possible to customize the primary key in LB3 so that we can have both int type id and string type uuid.

@agnes512 great! Now it works, thanks for your help!

Here is the result
Annotazione 3

@agnes512
Copy link
Contributor

agnes512 commented Jan 3, 2020

Great!! :D

As for your first definition, I think the problem happens when you do the creation.

customer.create({name: 'J D', uuid: 24});

Type of uuid is string. So it somehow assigns 24 to the int type id. And since the defaultFn of uuid is set, property uuid will be generated automatically, no need to set the value.

If you would like have auto increment id (as the id property) and auto-generated uuid, use the generated flag in your property definitions:

... customer mode def
  "properties": {
    "id": {
      "id": true,   // model primary key is still id 
      "generated": true,   // needed, but only works with int type id
      "type": "number"
    },
    "uuid": {
      "type": "string",
      "defaultFn": "uuidv4"
    },
   }..

with this definition, you can create instances without defining id or uuid:

customer.create({name: 'LoopBack'})

both id and uuid will be included in the response.

@fasenderos
Copy link
Author

fasenderos commented Jan 3, 2020

Great!! :D

As for your first definition, I think the problem happens when you do the creation.

customer.create({name: 'J D', uuid: 24});

Type of uuid is string. So it somehow assigns 24 to the int type id. And since the defaultFn of uuid is set, property uuid will be generated automatically, no need to set the value.

No no, I never create customer with the uuid prop. I always create customer only with the name but the response was an object with name and uuid (with the auto-incremented id - check the script at /boot/test.js here #4287 (comment))

However I understand the point. Thanks so much!

@agnes512
Copy link
Contributor

agnes512 commented Jan 7, 2020

Feel free to re-open the issue in case it is not solved yet.

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

4 participants