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

Maps saved as strings when the attribute is not defined in Schema and saveUnknown is true #236

Closed
stang-tgs opened this issue Nov 20, 2017 · 5 comments

Comments

@stang-tgs
Copy link
Contributor

stang-tgs commented Nov 20, 2017

When trying to save an item that contains a Map attribute that is not defined in the Schema, the Map is saved as a String, even though useDocumentTypes is set to true.

Also, nested map keys are also not saved if they aren't defined in the Schema, even if saveUnknown is true. See example:

const dynamoose = require('dynamoose');

const awsConfig = {
    region: 'us-west-2',
//    sslEnabled: false,
    endpoint: "http://localhost:8000"
};

dynamoose.local();

dynamoose.AWS.config.update(awsConfig);

const schema = new dynamoose.Schema({
    _id: {
        type: String, //guild id
        hashKey: true
    },
    aMap : {
        aKey : String
    }
}, {saveUnknown: true, useDocumentTypes: true});

const Room = dynamoose.model('Room', schema);
let room = new Room(
    {
        _id: "myRoom",
        aMap: {
            aKey: "valforkey",
            undefKey: "valForUnknown"
        },
        users: {
            apple: {name: "apple", score: 10},
            banana: {name: "banana", score: 20}
        }
    });
room.save().then(() => {
    return Room.get({_id: "myRoom"});
}).then((obj) => {
    console.log(obj);
}).catch(e => {console.log(e);});

Doing the query via aws-cli
aws dynamodb get-item --table-name Room --endpoint-url http://localhost:8000 --key '{"_id" : {"S" : "myRoom"}}'
we get:

{
    "Item": {
        "_id": {
            "S": "myRoom"
        }, 
        "aMap": {
            "M": {
                "aKey": {
                    "S": "valforkey"
                }
            }
        }, 
        "users": {
            "S": "{\"apple\":{\"name\":\"apple\",\"score\":10},\"banana\":{\"name\":\"banana\",\"score\":20}}"
        }
    }
}

Observe that "users" is a string, not a Map. Also, observe that "aMap" only has "aKey" set, even though we also wanted to set "undefkey".

Am I doing something wrong or is something not working as expected?

The reason why "users" being a Map is important is because I will be wanting to do updates to the "users" to add and remove individual users with 1 atomic update using conditions.

Thanks.

@brandongoode
Copy link
Contributor

Thanks for reporting the issue. I'll see if I can find a solution.

@stang-tgs
Copy link
Contributor Author

stang-tgs commented Nov 21, 2017

Thanks. I've done some investigation myself and have tracked it down to Attribute.setType(value). When parsing "unknown" attributes, the value is being passed in as a string from Schema.js Schema.toDynamo(model):

    if(!attr && this.options.saveUnknown) {
      attr = Attribute.create(this, name, typeof model[name]);
      this.attributes[name] = attr;
    }

After doing some reverse engineering, I believe the "normal" case for Attribute.setType(value) is when the type and schemas are explicitly defined, and that Attribute.setType(value) expects one of 2 forms: the raw value, or a type in string form.

Attribute.setType(value) has some conditionals to handle "special types" and mutate them:

  if (util.isArray(typeVal) && typeVal.length === 1 && typeof typeVal[0] === 'object'){
     type = 'List';
  } else if ( (util.isArray(typeVal) && typeVal.length === 1) || typeof typeVal === 'function') {
    this.isSet = util.isArray(typeVal);
    var regexFuncName = /^Function ([^(]+)\(/i;
    var found = typeVal.toString().match(regexFuncName);
    type = found[1];
  } else if (typeof typeVal === 'object'){
    type = 'Map';
  } else if (typeof typeVal === 'string') {
    type = typeVal;
  }

When an unknown attribute is passed in, the string "Object" will be passed in, and the type will NOT be set to 'Map'. This is why Unknown object types are always serialized as strings. (Aside: Unknown array types will also be wrong, since it'll be passed in as "Object")

I'm trying my hand in fixing this, but running into trouble since I'm still trying to understand the design and intention behind Attribute.setType and the overrides in the Attribute constructor.

  if(!schema.useDocumentTypes) {
    if(this.type.name === 'map') {
      debug('Overwriting attribute %s type to object', name);
      this.type = this.types.object;
    } else if (this.type.name === 'list') {
      debug('Overwriting attribute %s type to array', name);
      this.type = this.types.array;
    }
  }

Just posting my findings here so far so hopefully can fast track you or if you have any pointers.

In the meantime, I'll continue digging for a bit and update this comment if I find anything else or come up with a solution.

Cheers.

@stang-tgs
Copy link
Contributor Author

stang-tgs commented Nov 21, 2017

I might have a fix. Please see pull request #240

P.S. nested unknown attributes are still not set and not addressed in this PR. TBD on what the intended behaviour is? Currently, for unknown attributes to populate properly, the unknown attribute must be at the topmost level of the Model and cannot be defined in the Schema, or it will not be written.

@jignesh248
Copy link

"nested unknown attributes are still not set and not addressed in this PR", I want to save nested unknown attributes 2 levels deep in an object. but it seems they aren't being saved as of now.

{
    topLevelKey:{
        secondLevel:{
            unknownGeneratedID1: { some key/value pairs },
            unknowGeneratedID2:{ some key/value pairs}
        }
    }
}

@fishcharlie
Copy link
Member

@jignesh248 I have moved that into a separate issue #323. A PR would always be appreciated to address this feature to get this integrated.

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