hasMany through doesn't set the through key value #394

clarkbw · 2015-01-15T07:05:57Z

I created a quick test to describe the hasManyThrough issue I'm seeing. I left in the console.log call so you can easily see the problematic object. (pull in my PR and run npm test)

{ "followerId": 2,
  "date": "Wed Jan 14 2015 22:54:43 GMT-0800 (PST)",
  "id": 1 }

followerId is being set but the followeeId attribute is not set; a critical piece when creating this object. This test demonstrates using the add function though I haven't testing with other relation functions like create.

slnode · 2015-01-15T07:05:58Z

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

clarkbw · 2015-01-16T00:03:51Z

I did a little digging, I'm guessing the throughKeys function is not correctly returning keys for a hasManyThrough relation. Might be able to look into a fix later but I don't understand all of what the throughKeys function does.
https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/relation-definition.js#L816

Logging the keys returned from within the add function shows the wrong set returned.
https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/relation-definition.js#L979

raymondfeng · 2015-01-16T00:21:14Z

There seems to be a bug in handling multiple belongsTo relations to the same model, see:
https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/relation-definition.js#L464

steirico · 2015-01-16T09:52:21Z

I'm facing this problem (or a related one) with loopback-datasource-juggler 2.14.1, too.

I have the following models:

{
  "name": "Customuser",
  "base": "User",
  "properties": {
.....
  },
  "relations": {
    "supportingUsers": {
      "type": "hasMany",
      "model": "Customuser",
      "through": "SupporterMapping",
      "foreignKey": "supportingUserId"
    },
    "supportedUsers": {
      "type": "hasMany",
      "model": "Customuser",
      "through": "SupporterMapping",
      "foreignKey": "supportedUserId"
    },
    "supportedSites": {
      "type": "hasMany",
      "model": "Site",
      "through": "SupporterMapping",
      "foreignKey": "supportedSiteId"
    },
  },
...
}

{
  "name": "Site",
  "base": "PersistedModel",
  "properties": {
...
  },
  "relations": {
    "customer": {
      "type": "belongsTo",
      "model": "Customuser",
      "foreignKey": "customerId"
    },
    "siteSupporter": {
      "type": "hasMany",
      "model": "Customuser",
      "through": "SupporterMapping",
      "foreignKey": "supportingUserId"
    }
  },
...
}

{
  "name": "SupporterMapping",
  "base": "PersistedModel",
  "properties": {
...
  },
  "relations": {
    "supportingUser": {
      "type": "belongsTo",
      "model": "Customuser"
    },
    "supportedUser": {
      "type": "belongsTo",
      "model": "Customuser"
    },
    "supportedSite": {
      "type": "belongsTo",
      "model": "Site"
    }
  },
...
}

IMHO, relations are set up correctly. Querying the model SupporterMapping using the StrongLoop API Explorer resolves all relations correctly.
GET /SupporterMappings/{id}/supportedSite -> ok
GET /SupporterMappings/{id}/supportedUser -> ok
GET /SupporterMappings/{id}/supportingUser -> ok

But errors arise for the following API methods:
GET /Customuser/{id}/supportedSites

{
  "error": {
    "name": "Error",
    "status": 500,
    "message": "Relation \"site\" is not defined for SupporterMapping model",
    "stack": "Error: Relation \"site\" is not defined for SupporterMapping model\n    at processIncludeItem (c:\\hfb_webapp\\node_modules\\loopback-datasource-juggler\\lib\\include.js:144:10)\n    at c:\\hfb_webapp\\node_modules\\loopback-datasource-juggler\\lib\\include.js:118:5\n    at c:\\hfb_webapp\\node_modules\\async\\lib\\async.js:125:13\n    at Array.forEach (native)\n    at _each (c:\\hfb_webapp\\node_modules\\async\\lib\\async.js:46:24)\n    at Object.async.each (c:\\hfb_webapp\\node_modules\\async\\lib\\async.js:124:9)\n    at Function.Inclusion.include (c:\\hfb_webapp\\node_modules\\loopback-datasource-juggler\\lib\\include.js:117:9)\n    at MySQL.<anonymous> (c:\\hfb_webapp\\node_modules\\loopback-connector-mysql\\lib\\mysql.js:588:33)\n    at releaseConnectionAndCallback (c:\\hfb_webapp\\node_modules\\loopback-connector-mysql\\lib\\mysql.js:149:17)\n    at Query.<anonymous> (c:\\hfb_webapp\\node_modules\\loopback-connector-mysql\\lib\\mysql.js:163:7)"
  }
}

GET /Hoocusers/{id}/supportedUsers

{
  "error": {
    "name": "Error",
    "status": 500,
    "message": "Relation \"customuser\" is not defined for SupporterMapping model",
    "stack": "Error: Relation \"customuser\" is not defined for SupporterMapping model\n    at processIncludeItem (c:\\hfb_webapp\\node_modules\\loopback-datasource-juggler\\lib\\include.js:144:10)\n    at c:\\hfb_webapp\\node_modules\\loopback-datasource-juggler\\lib\\include.js:118:5\n    at c:\\hfb_webapp\\node_modules\\async\\lib\\async.js:125:13\n    at Array.forEach (native)\n    at _each (c:\\hfb_webapp\\node_modules\\async\\lib\\async.js:46:24)\n    at Object.async.each (c:\\hfb_webapp\\node_modules\\async\\lib\\async.js:124:9)\n    at Function.Inclusion.include (c:\\hfb_webapp\\node_modules\\loopback-datasource-juggler\\lib\\include.js:117:9)\n    at MySQL.<anonymous> (c:\\hfb_webapp\\node_modules\\loopback-connector-mysql\\lib\\mysql.js:588:33)\n    at releaseConnectionAndCallback (c:\\hfb_webapp\\node_modules\\loopback-connector-mysql\\lib\\mysql.js:149:17)\n    at Query.<anonymous> (c:\\hfb_webapp\\node_modules\\loopback-connector-mysql\\lib\\mysql.js:163:7)"
  }
}

GET /Hoocusers/{id}/supportingUsers

{
  "error": {
    "name": "Error",
    "status": 500,
    "message": "Relation \"customuser\" is not defined for SupporterMapping model",
    "stack": "Error: Relation \"customuser\" is not defined for SupporterMapping model\n    at processIncludeItem (c:\\hfb_webapp\\node_modules\\loopback-datasource-juggler\\lib\\include.js:144:10)\n    at c:\\hfb_webapp\\node_modules\\loopback-datasource-juggler\\lib\\include.js:118:5\n    at c:\\hfb_webapp\\node_modules\\async\\lib\\async.js:125:13\n    at Array.forEach (native)\n    at _each (c:\\hfb_webapp\\node_modules\\async\\lib\\async.js:46:24)\n    at Object.async.each (c:\\hfb_webapp\\node_modules\\async\\lib\\async.js:124:9)\n    at Function.Inclusion.include (c:\\hfb_webapp\\node_modules\\loopback-datasource-juggler\\lib\\include.js:117:9)\n    at MySQL.<anonymous> (c:\\hfb_webapp\\node_modules\\loopback-connector-mysql\\lib\\mysql.js:588:33)\n    at releaseConnectionAndCallback (c:\\hfb_webapp\\node_modules\\loopback-connector-mysql\\lib\\mysql.js:149:17)\n    at Query.<anonymous> (c:\\hfb_webapp\\node_modules\\loopback-connector-mysql\\lib\\mysql.js:163:7)"
  }
}

GET /Sites/{id}/siteSupporter

{
  "error": {
    "name": "Error",
    "status": 500,
    "message": "Relation \"customuser\" is not defined for SupporterMapping model",
    "stack": "Error: Relation \"customuser\" is not defined for SupporterMapping model\n    at processIncludeItem (c:\\hfb_webapp\\node_modules\\loopback-datasource-juggler\\lib\\include.js:144:10)\n    at c:\\hfb_webapp\\node_modules\\loopback-datasource-juggler\\lib\\include.js:118:5\n    at c:\\hfb_webapp\\node_modules\\async\\lib\\async.js:125:13\n    at Array.forEach (native)\n    at _each (c:\\hfb_webapp\\node_modules\\async\\lib\\async.js:46:24)\n    at Object.async.each (c:\\hfb_webapp\\node_modules\\async\\lib\\async.js:124:9)\n    at Function.Inclusion.include (c:\\hfb_webapp\\node_modules\\loopback-datasource-juggler\\lib\\include.js:117:9)\n    at MySQL.<anonymous> (c:\\hfb_webapp\\node_modules\\loopback-connector-mysql\\lib\\mysql.js:588:33)\n    at releaseConnectionAndCallback (c:\\hfb_webapp\\node_modules\\loopback-connector-mysql\\lib\\mysql.js:149:17)\n    at Query.<anonymous> (c:\\hfb_webapp\\node_modules\\loopback-connector-mysql\\lib\\mysql.js:163:7)"
  }
}

So far, I was able to track down the problem to this section where relationName is not resolved correctly:
https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/include.js#L129-L140

Hopefully, this is helpful for you. Thanks for your excellent work!

clarkbw · 2015-01-16T17:13:51Z

Here's my solution so far: (code not ready for check-in)
Changed the findBelongsTo function so it returns an array of options found. From the throughKeys function you can then test to see what type of relations you're working with and use the appropriate keys returned. This doesn't scale to other cases where you might have multiple belongsTo relations but only want a specific set.

raymondfeng · 2015-01-19T23:38:57Z

@clarkbw Pls let us know when your patch is ready to merge.

steirico · 2015-01-29T15:35:48Z

I just want to let you know that @clarkbw's patch is working for me.

Furthermore, I had to add the "keyThrough" property to the "hasMany" relation definitions. Unfortunately, this property is not documented anywhere and it cost me half a day digging into the code and find this solution. Enhance the docs accordingly, please.

But now, my models work now as expected. Here my modified model definitions. Hopefully, someone can benefit from them:

{
  "name": "Customuser",
  "base": "User",
  "properties": {
.....
  },
  "relations": {
    "supportingUsers": {
      "type": "hasMany",
      "model": "Hoocuser",
      "through": "SupporterMapping",
      "foreignKey": "supportedUserId",
      "keyThrough": "supportingUserId"
    },
    "supportedUsers": {
      "type": "hasMany",
      "model": "Hoocuser",
      "through": "SupporterMapping",
      "foreignKey": "supportingUserId",
      "keyThrough": "supportedUserId"
    },
    "supportedSites": {
      "type": "hasMany",
      "model": "Site",
      "through": "SupporterMapping",
      "foreignKey": "supportingUserId",
      "keyThrough": "supportedSiteId"
    },
  },
...
}

{
  "name": "Site",
  "base": "PersistedModel",
  "properties": {
...
  },
  "relations": {
    "customer": {
      "type": "belongsTo",
      "model": "Customuser",
      "foreignKey": "customerId"
    },
    "siteSupporters": {
      "type": "hasMany",
      "model": "Hoocuser",
      "through": "SupporterMapping",
      "foreignKey": "supportedSiteId",
      "keyThrough": "supportingUserId"
    }
  },
...
}

{
  "name": "SupporterMapping",
  "base": "PersistedModel",
  "properties": {
...
  },
  "relations": {
    "supportingUser": {
      "type": "belongsTo",
      "model": "Customuser"
    },
    "supportedUser": {
      "type": "belongsTo",
      "model": "Customuser"
    },
    "supportedSite": {
      "type": "belongsTo",
      "model": "Site"
    }
  },
...
}

clarkbw · 2015-02-01T05:00:47Z

@raymondfeng let me know if including lodash-node is ok. I can make it work either way; I prefer using lodash but I'm not sure what the project guidelines are for module inclusion.

raymondfeng · 2015-02-01T06:16:27Z

lodash is fine.

clarkbw · 2015-02-02T08:19:36Z

@raymondfeng updated, rebased against master and squashed into 1 commit. I'm probably going to continue to be slow on the turnaround time but let me know if you want any changes.

raymondfeng · 2015-02-02T17:07:02Z

ok to test

raymondfeng · 2015-02-09T22:58:51Z

@clarkbw Have you signed the CLA - https://cla.strongloop.com/agreements/strongloop/loopback-datasource-juggler?

clarkbw · 2015-02-10T05:43:30Z

Have you signed the CLA - https://cla.strongloop.com/agreements/strongloop/loopback-datasource-juggler?

Sorry. I thought I did, but I'm wondering now if I signed it for loopback and not this module if they are different CLAs. Signed again, I'll look into the build failures.

raymondfeng · 2015-02-12T04:16:34Z

@clarkbw Any update on the test failures?

clarkbw · 2015-02-16T18:56:19Z

@clarkbw Any update on the test failures?

Looking through them now, I wasn't seeing those previously so I'll have to track down the change that made the difference.

…matches

clarkbw · 2015-02-17T05:56:49Z

@raymondfeng updated and passing.
The change from lodash-node to lodash caused a little confusion, all fixed now. I'm still not totally happy with it, the code isn't any easier to read; it would be nice to do a bit of refactor cleanup that created some sub-modules to separate concerns.

hasMany through doesn't set the through key value

AurelieV · 2015-04-03T10:41:58Z

@steirico I have the same problem as you, but your synthax did not worked for me.
One side works well, but other side has a very strange behavior (returning doublons, even all data).
Which version to you use? Which database?
Thanks for your answer

clarkbw · 2015-04-03T22:42:41Z

but other side has a very strange behavior (returning doublons, even all data).

@AurelieV do you have a test case you can share so I can take a look?

I'm using MongoDB and you'll need this module as of 2.18.0 when the fix landed.

AurelieV · 2015-04-08T08:24:24Z

@clarkbw I use the same configuration than you.
I have finally discovered that the problem only appends when I add some fields filter in my request.
Fetch with only include works fine, but not any more with fields.
I load links between Definitions (same as Supporters in your case) manually from an external source. Then I check the through table, and everythink is ok, but problems happens when I check getting related definitions (referenced and referencing) for a given definition.
(coffee synthax)

(working, defs contains what I want)

app.models.Definition.findOne { where: { external_id: '19A35136542D582B' }, include: ['referencedDefinitions']}, (err, def) ->
  assert.equal err, null
  defs = def.referencedDefinitions()

(not working, defs contains doublons or even all data)

app.models.Definition.findOne { where: { external_id: '19A35136542D582B' }, include: ['referencedDefinitions'], fields:['referencedDefinitions']}, (err, def) ->
  assert.equal err, null
  defs = def.referencedDefinitions()

altsang added the #community contribution label Jan 15, 2015

bajtos assigned raymondfeng Jan 19, 2015

raymondfeng mentioned this pull request Jan 22, 2015

Can't Use HasManyThrough When The From And To Models Are The Same strongloop/loopback#859

Closed

use lodash to update the findBelongsTo which now returns an array of …

98fcf6b

…matches

raymondfeng added a commit that referenced this pull request Feb 17, 2015

Merge pull request #394 from clarkbw/hasManyThroughKeyThrough

a44e45b

hasMany through doesn't set the through key value

raymondfeng merged commit a44e45b into loopbackio:master Feb 17, 2015

raymondfeng removed the #community contribution label Feb 17, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hasMany through doesn't set the through key value #394

hasMany through doesn't set the through key value #394

clarkbw commented Jan 15, 2015

slnode commented Jan 15, 2015

clarkbw commented Jan 16, 2015

raymondfeng commented Jan 16, 2015

steirico commented Jan 16, 2015

clarkbw commented Jan 16, 2015

raymondfeng commented Jan 19, 2015

steirico commented Jan 29, 2015

clarkbw commented Feb 1, 2015

raymondfeng commented Feb 1, 2015

clarkbw commented Feb 2, 2015

raymondfeng commented Feb 2, 2015

raymondfeng commented Feb 9, 2015

clarkbw commented Feb 10, 2015

raymondfeng commented Feb 12, 2015

clarkbw commented Feb 16, 2015

clarkbw commented Feb 17, 2015

AurelieV commented Apr 3, 2015

clarkbw commented Apr 3, 2015

AurelieV commented Apr 8, 2015

hasMany through doesn't set the through key value #394

hasMany through doesn't set the through key value #394

Conversation

clarkbw commented Jan 15, 2015

slnode commented Jan 15, 2015

clarkbw commented Jan 16, 2015

raymondfeng commented Jan 16, 2015

steirico commented Jan 16, 2015

clarkbw commented Jan 16, 2015

raymondfeng commented Jan 19, 2015

steirico commented Jan 29, 2015

clarkbw commented Feb 1, 2015

raymondfeng commented Feb 1, 2015

clarkbw commented Feb 2, 2015

raymondfeng commented Feb 2, 2015

raymondfeng commented Feb 9, 2015

clarkbw commented Feb 10, 2015

raymondfeng commented Feb 12, 2015

clarkbw commented Feb 16, 2015

clarkbw commented Feb 17, 2015

AurelieV commented Apr 3, 2015

clarkbw commented Apr 3, 2015

AurelieV commented Apr 8, 2015