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

Add possibility to parse/convert property values #8

Open
VividVisions opened this issue Jun 15, 2016 · 7 comments
Open

Add possibility to parse/convert property values #8

VividVisions opened this issue Jun 15, 2016 · 7 comments

Comments

@VividVisions
Copy link

I needed a way to convert some properties, after fetching them from the DB. And since I wanted to prevent traversing the whole result set (before JoinJS) or object tree (after JoinJS) again, I thought it'd be great, if JoinJS could handle this in one go. So, I played around with something like this in injectResultInObject:

// Copy id property
//[…]

let properties = {};

// Gather other properties
_.each(resultMap.properties, function(property) {
    // If property is a string, convert it to an object
    if (typeof property === 'string') {
        property = {name: property, column: property};
    }

    // Copy only if property does not exist already
    if (!mappedObject[property.name]) {
        // The default for column name is property name
        let column = (property.column) ? property.column : property.name;
        properties[property.name] = result[columnPrefix + column];
        //mappedObject[property.name] = result[columnPrefix + column];
    }
});

// Check for conversion
if (resultMap.parse) {
    properties = resultMap.parse(properties);
}

// Copy other properties
_.merge(mappedObject, properties);

// Copy associations
// […]

The reason I don't pass the whole mappedObject to the parse function is, that this way, the ID property and any other properties or functions the mappedObject could have when createNew is used are omitted and the chances of completely messing things up during parsing/converting are lower. ;)

What do you think? Would this be something you see within the scope of JoinJS?

@FerrielMelarpis
Copy link
Contributor

@VividVisions
I don't know if I got your query right.
I'm assuming you want to have an option for parsing properties once the result has been mapped to the resultMap.
I would like to know the use-case for this.
I have a suggestion though that may be beneficial to you.
Let's say we have a result set like this:

const resultSet = [
    { id: 1, json: '{"a": 1}', ... } // possible when using mysql json type
];

and you would want to parse the JSON string to a JSON object using JSON.parse

const resultMap = [{
    mapId: 'jsonMap',
    idProperty: 'id',
    properties: [
        { 
            name: 'json', 
            column: 'json', 
            parse(value) {
                try {
                    return JSON.parse(value);
                } catch (error) {
                    return value;
                }
            }
        },
        ...
    ]
}];

Instead of parsing the list of properties you can pinpoint what property needs to be parsed.

@nareshbhatia What do you think about this option?

@VividVisions
Copy link
Author

@FerrielMelarpis

I'm assuming you want to have an option for parsing properties once the result has been mapped to the resultMap.

Exactly.

One of my use cases is converting binary hashes (DB side) to hex representations (API/UI side) and back. Another one is creating "virtual" properties out of a couple of DB-stored ones. Simplified example would be:

parse: properties => {
   properties.fullName = `${properties.firstName} ${properties.lastName}`;
   return properties;
}

While your suggestion would be great for the first use case, the second one would not be feasible without traversing the mapped result another time, which I wanted to prevent in the first place.

In my case it's also a lot easier to use just one parse function for the whole list/object because the resultMap is dynamically generated according to my model definitions. Having to create a function for each property to convert would be a lot more work. (I've been using my variant for a while now in a production environment.)

@FerrielMelarpis
Copy link
Contributor

@VividVisions
I see. Your idea is better and easier to implement. Wait for @nareshbhatia if he wants to include this for the stable version.

@nareshbhatia
Copy link
Member

Good discussion, @VividVisions and @FerrielMelarpis. I agree that Walter's suggestion would be more flexible. Only suggestion from my side is to call this ResultMap property transform instead of parse - if present it allows you to transform the properties. If that makes sense, Walter, I would welcome a PR prom you.

@Mikou
Copy link

Mikou commented Oct 19, 2018

First of, thanks for this library, it's great.

I thought that I would also need such a transform method to handle the conversion between snake_case and camelCase that differs between my db and app namings.
This was until I realized that propertyIds and properties can be defined as {name: 'myCamelCaseVar': column: 'my_snake_case_var'}. This solves the problem :-)

What if I want to hash the value of "id" values returned from the db? Is there an existing way to do this during joinjs.map() or should I do it as an extra step on each object in the resulting mapped collection?

@FerrielMelarpis
Copy link
Contributor

@Mikou solving this issue might solve your problem after all since basically, transform is a function applied after mapping the properties. Although I suggest injecting a new field instead of overriding the id. I'm not sure about your use case though but for most case, you would still need the id e.g. indexing.
The code might look like this:

transform: props => {
  props.hash = hash(props.id);
  return props;
}

I'll take a look on this when I got time.

@Mikou
Copy link

Mikou commented Oct 19, 2018

Hi @FerrielMelarpis,

Thanks for your answer. You are right, injecting a new field would be a better idea than overriding the id.

My use case concerning hashing the id, is simply to "hide" the real database id (that is an auto-increment integer) behind a hashed value using the hashids library.

Maybe this is overkilled but what about adding 2 functions to resultMap instead of just one: transform and extend.

transform would be applied for each propertyId and each properties and would receive as a parameter it's relating name from the resultset. This allows to transform the result fields in the resultSet.

This would allow to succinctly describe transformation rules such as db's snake_case to app's camelCase naming convention for all fields like so:

    {
        mapId: 'transformMap',
        idProperty: ['myId'],
        properties: [
            'myFirstKey',
            'mySecondKey'
        ],
        transform: _.camelCase
    }

extend would be called in a second time (after transformation) mainly to allow adding extra fields to the object like so:

    {
        mapId: 'extendMap',
        properties: ['firstname', 'lastname'],
        extend: properties =>
            Object.assign(properties, {
                fullname: properties.firstname + ' ' + properties.lastname
            })
    }

The extend function can also be used to rename the keys. However, it feels like transforming and extending belong to their own separate concerns. Also, within an application, in many cases extending will probably not be used whereas transforming might be used for every single db resultset if there is a naming convention mismatch between the db and the app (camelCase -> snake_case, db prefix etc...). In this case, it is shorter to just write transform: _.camelCase.

I have tried to illustrate precisely this in a fork I have created:
https://github.com/Mikou/joinjs/blob/c1b8f79edf0780d9d262aedb2aa4034ed26db337/test/maps/test-maps.js#L178-L217

NB: Currently, without the "transform" function, I solve camelCase/snakeCase problem like so:

const fields = [...];
export const relationMap = {
  mapId: "eventMap",
  idProperty: "id",
  // properties: [...fields.map(_.snakeCase)],
  properties: [
    ...fields.map(field => ({
      name: field,
      column: _.snakeCase(field) // <- I convert the column to match the result field instead of converting the result field to match the column.
    }))
  ],
  associations: [{ name: "author", mapId: "userMap", columnPrefix: "user_" }]
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants