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

Transformation.prototype.unserialize | unserialize the row from DB is not working correctly #4079

Closed
Seafnox opened this issue Apr 20, 2017 · 10 comments

Comments

@Seafnox
Copy link

Seafnox commented Apr 20, 2017

Sails version: 0.12.11 and 1.0
Node version: 7.4
NPM version: 3.10.9
Operating system: Windows


When model contain some transformations by `columnName` it can raise disapearing columns with values.
If column names from thee DB (Mongo, MySQL, PostgreSQL, etc.) intersect with field names for user like:
module.exports = {
  attributes: {
    identity: {
      columnName: 'aid',
    },
    ownerId: {
      columnName: 'identity',
    },
    .....
  }
}

then column ownerId resieve right value, but identity value with column was disappearing:

[LOG] Transformation.prototype.unserialize
[LOG] INCOMING DATA: {"aid":120,"name":"explicabo","restriction":65535,"identity":57205}
[LOG] OUTCOMMING DATA: {"restriction":65535,"name":"explicabo","ownerid":57205} - "id" no more exist

For any cases, when column names are intersect like:
a -> b
b -> c
system can loose one of columns

@sailsbot
Copy link

Hi @Seafnox! It looks like you may have removed some required elements from the initial comment template, without which I can't verify that this post meets our contribution guidelines. To re-open this issue, please copy the template from here, paste it at the beginning of your initial comment, and follow the instructions in the text. Then post a new comment (e.g. "ok, fixed!") so that I know to go back and check.

Sorry to be a hassle, but following these instructions ensures that we can help you in the best way possible and keep the Sails project running smoothly.

*If you feel this message is in error, or you want to debate the merits of my existence (sniffle), please contact inquiries@sailsjs.com

@sailsbot
Copy link

Sorry to be a hassle, but it looks like your issue is still missing some required info. Please double-check your initial comment and try again.

*If you feel this message is in error, or you want to debate the merits of my existence (sniffle), please contact inquiries@sailsjs.com

@sailsbot
Copy link

@Seafnox Thanks for posting, we'll take a look as soon as possible.


For help with questions about Sails, click here. If you’re interested in hiring @sailsbot and her minions in Austin, click here.

@Seafnox
Copy link
Author

Seafnox commented Apr 20, 2017

I find solution: Instead rewriting exsisting object in Transformation.prototype.unserialize system must create new empty one and unserialize columns from incoming object to created use transformations rules

@Seafnox
Copy link
Author

Seafnox commented Apr 20, 2017

waterline/lib/waterline/utils/system/transformer-builder.js

...
Transformation.prototype.unserialize = function(pRecord) {
  let uRecord = {}
  // Loop through the transformation keys for the record.
  _.each(this._transformations, function(columnName, attrName) {

    if (!_.has(pRecord, columnName)) {
      return;
    }

    uRecord[attrName] = pRecord[columnName];
    delete pRecord[columnName];

  });
  // Merge untransformated columns.
 uRecord = _.extend(uRecord, pRecord);

  return uRecord;
};

@sgress454
Copy link
Member

Hi @Seafnox -- thanks for this. Do you think you could make a pull request to Waterline that explains this problem in more detail and includes the fix?

@Seafnox
Copy link
Author

Seafnox commented May 8, 2017

i'll try

@sailsbot
Copy link

sailsbot commented Jun 8, 2017

@Seafnox,@sailsbot,@sgress454: Hello, I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message and simply close the issue if possible. On the other hand, if you are still waiting on a patch, please post a comment to keep the thread alive (with any new information you can provide).

If no further activity occurs on this thread within the next 3 days, the issue will automatically be closed.

Thanks so much for your help!

@sailsbot sailsbot added waiting to close This label is deprecated. Please don't use it anymore. and removed waiting to close This label is deprecated. Please don't use it anymore. labels Jun 8, 2017
@sailsbot sailsbot added the waiting to close This label is deprecated. Please don't use it anymore. label Jul 10, 2017
@sailsbot
Copy link

@Seafnox,@sailsbot,@sgress454: Hello, I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message and simply close the issue if possible. On the other hand, if you are still waiting on a patch, please post a comment to keep the thread alive (with any new information you can provide).

If no further activity occurs on this thread within the next 3 days, the issue will automatically be closed.

Thanks so much for your help!

@sailsbot sailsbot removed the waiting to close This label is deprecated. Please don't use it anymore. label Jul 14, 2017
sgress454 added a commit to balderdashy/waterline that referenced this issue Aug 7, 2017
sgress454 added a commit to balderdashy/waterline that referenced this issue Aug 7, 2017
…'s `columnName`

Note that in contrast to the v0.13.x version of this patch, in this case we are _not_ modifying the original record in place.  This is for consistency with how it has been working in v0.11.x (see https://github.com/balderdashy/waterline/blob/0.11.x/lib/waterline/core/transformations.js#L154)

refs balderdashy/sails#4079
sgress454 added a commit to balderdashy/waterline that referenced this issue Aug 8, 2017
…transforming.

Previously, only the last join in the joins array got the `serializeCriteria` treatment, ostensibly because the parent join in a many-to-many didn't need it?  But this broke down for queries with multiple populates; only the last of the joins would get transformed.  Furthermore, when the joins were being generated, they were already using `columnName` in their `select` clauses, so that when `serializeCriteria` ran it could cause issues if there were conflicts between attribute names and column names (see balderdashy/sails#4079).  With this patch, only attribute names are used in the `select` clauses for joins, and then _all_ joins are transformed.  Note that in the case of many-to-many joins, the first ("parent") join is expected to have `select: false` and no criteria object.  It's not clear why this should be (I think `select: []` and `criteria: {}` should work just as well, and tests pass that way), but to avoid unforeseen issues, backwards-compatibility for that has been added in lines 609-612.

refs #1499
@sailsbot
Copy link

sailsbot commented Sep 7, 2017

@Seafnox,@sailsbot,@sgress454: Hello, I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message and simply close the issue if possible. On the other hand, if you are still waiting on a patch, please post a comment to keep the thread alive (with any new information you can provide).

If no further activity occurs on this thread within the next 3 days, the issue will automatically be closed.

Thanks so much for your help!

@sailsbot sailsbot added waiting to close This label is deprecated. Please don't use it anymore. and removed waiting to close This label is deprecated. Please don't use it anymore. labels Sep 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants