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

push/pop-like operator for array based fields #39

Closed
Taik opened this issue Jul 1, 2015 · 26 comments
Closed

push/pop-like operator for array based fields #39

Taik opened this issue Jul 1, 2015 · 26 comments

Comments

@Taik
Copy link

Taik commented Jul 1, 2015

Currently, the only way to work with array-type fields is to explicitly specify them by index:

Let c be:
{
  members: [
    'member-1',
    'member-2',
    'member-3'
  ]
}

c.get('members.0') // Returns 'member-1'
c.set('members.4', 'member-4')

It would be nice to have a way to push a new element onto the array like so:

c.push('members', 'member-4')
c.pop('members') // Returns 'member-4'
c.pop('members', 0) // Returns 'member-1'
@lukejagodzinski
Copy link
Member

I was thinking about solving this problem and introducing push and pop methods is not enough. We would need all the array methods. So it's not a good solution to duplicate them. Right now you can do as follows:

c.set('members', []);
c.members.push({
  name: 'Name'
});

It will still detect the changes and update them. There is also one more feature that needs implementing. When a new member gets pushed into the array and the document is saved then all members will be saved to the database (not just the one that was pushed). I have to implement detection of changes (diff between arrays).

@Taik
Copy link
Author

Taik commented Jul 2, 2015

Thanks for the workaround; That will suffice for me at the moment as I don't foresee my arrays growing more than a set limit. But for future optimization, diffing between arrays would be ideal to reduce I/O in mongo.

@lukejagodzinski
Copy link
Member

Yes right. I definitely have to figure something out.
Regards

@JulianKingman
Copy link

That workout didn't work for me? When I try it I get this:
members: { 0: { name: 'Name'}}

Shouldn't it be: members: { 'Name' } ?

I'm trying to add an item to an array, it's proving difficult, do I need to use collection.update on this? One great thing about astronomy that I've really liked is not having to write these methods.

@lukejagodzinski
Copy link
Member

Could you paste here the code that is not working for you?

@JulianKingman
Copy link

Hi Jagi,

    found.set('source', []);
       found.source.push({
         name: source._id
      });
    found.save();

On August 3, 2015 at 4:12:24 AM, jagi (notifications@github.com) wrote:

Could you paste here the code that is not working for you?


Reply to this email directly or view it on GitHub.

@lukejagodzinski
Copy link
Member

When you have class / model:

Founds = new Mongo.Collection('founds');
Found = Astro.Class({
  collection: Founds,
  fields: {
    source: {
      type: 'array',
      default: []
    }
  }
});

then you can do as follows:

var found = new Found();
found.set('source', []); // This line is necessary because we defined default value for the "source" field.
found.source.push({
  name: 'abc'
});
found.save();

The found.source will be equal [{name: 'abc'}].

Is this exactly what you expect?

@JulianKingman
Copy link

That's what I'm going for, how do I get it to work for 'updates'? It seems like this would overwright the array (with found.set('source', []))

@lukejagodzinski
Copy link
Member

With updates it works as well but you don't need to found.set('source', []);.

You just write:

var found = Founds.findOne();
found.source.push({
  name: 'abc'
});
found.save();

That's it

@JulianKingman
Copy link

That's great, so then I assume if it's a non-keyed array, it would looke like this: found.source.push('newarrayvalue')?

@lukejagodzinski
Copy link
Member

Yes exactly

@JulianKingman
Copy link

This seems to get a little funny with the Meteor.users collection. For one, the collection only allows modifying emails, services, and profiles, so I had to re-work my schema a bit to not get permission errors.
The other thing is, if I store fields in the 'profile' object, even setting a default [] value to an array doesn't create the profile or the array when a user is added. It also doesn't seem to create the array when I do user.profile.set('someArray',[]) (note that user.set('someObject', {}) DOES work).
Looking through issues, it looks like the 'default' portion is related to issue #54.

@lukejagodzinski
Copy link
Member

Instead writing:

user.profile.set('someArray', []);

you have to write:

user.set('profile.someArray', []);

In your schema, you also have to define it like this:

User = Astro.Class({
  /* ... */
  fields: {
    'profile': {
      type: 'object',
      default: {}
    },
    'profile.someArray': {
      type: 'array',
      default: []
    }
  }
});

Maybe it's seems to be a little bit not intuitive. However in the next release, it will change a little bit to something like this:

This code is for the version 0.13.0 (NOT YET RELEASED)

Profile = Astro.Class({
  name: 'Profile',
  fields: {
    name: {
      type: 'String'
    },
    someArray: {
      type: 'Array',
      default: []
    }
  }
});

User = Astro.Class({
  name: 'User',
  collection: Meteor.users,
  fields: {
    emails: 'Array',
    services: 'Object',
    createdAt: 'Date',
    profile: {
      type: 'Profile',
      default: {}
    }
  }
});

Thanks to that you will have more control over your data and it will be possible to do something like this:

user.profile.set('someArray', []);

as well as this:

user.set('profile.someArray', []);

@JulianKingman
Copy link

Huh, I could have sworn I tried that, but I just switched it to what you suggested and it worked perfectly.

I'm sure the new way will be very powerful, but it looks more confusing to me... (note: I'm pretty new to this stuff, so take it with a grain of salt.) What does it mean to have nested Astro classes? Can you embed another class inside that one? Is it basically creating a reusable schema, and can you use a class to extend another one, or only to define fields?

Thanks!

@lukejagodzinski
Copy link
Member

I'm working on this new release because I want Astronomy to work not only with MongoDB but also with other databases also relational. In most relational databases you can have nested objects or arrays like for instance the Meteor.users collection.

var user = {
  username: 'jagi',
  emails: [ // In SQL databases it would be moved to external table
    { address: "cool@example.com", verified: true },
    { address: "another@different.com", verified: false }
  ],
  profile: {}, // In SQL databases it would be moved to external table
  services: {} // In SQL databases it would be moved to external table
};

However in MongoDB or RethinkDB it's possible. So my solution for Astronomy to make it work with other databases is Nested classes.

For example you define that emails are of the Email type. In MongoDB it will be saved just in the user object, however in the SQL it would be saved in the external table. So it's how I'm gonna achieve database abstraction.

Moreover it will be easier to perform many operations because classes are gonna be flat (set of first class fields - no nested fields definition). Nested fields definition will be achieved by using nested classes. I hope it explains everything. If not, let me know I will try to describe it better :)

@JulianKingman
Copy link

That makes sense, it's very forward-thinking. What about other class attributes, like what would happen if you put methods on a class that you nested as a field type? That's what really confuses me, that you use the same class to define embedded fields as you do to define the collection objects. On the one hand, it's a very elegant approach, but on the other it seems a bit unstructured, at least visually.
You could do something to clarify that there's a difference with the class, maybe just adding a 'type: field' attribute to the Astro.Class. It doesn't necessarily have to do anything, or perhaps it can disable other class attributes besides fields. Or you could call the class Astro.Field, or something. Just some thoughts.

@lukejagodzinski
Copy link
Member

The difference is in the presence of collection attribute which tells that the class should be persisted in the database. There was an idea to call it just a schema. And compose class from one or many schemas.

@JulianKingman
Copy link

I'll just wait and see :)

@ghost
Copy link

ghost commented Aug 25, 2015

I've got a question. If i have a field of type array, which in this case, represent the history of the class (for example, an history of the change in prices for an article...). I don't want to pull the full history in memory every time the user is gonna make a change, it could become heavy with time... And because of this, if y try to do in an update :

this.field.push({
   price: priceValue
});

It won't work has it's gonna override the values already there...

@lukejagodzinski
Copy link
Member

var product = Products.findOne();
product.history.push({
  price: priceValue
});
product.save();

The save method will save entire history array, not only the last value. If the list of the operations is long it may not be a good choice to use product.save() to update only this fields. In such situation it would be better to use:

Products.update(product._id, {
  $push: {
    history: {price: priceValue}
  }
});

However, in this situation you have to create your own validation logic. With the new Astronomy release there will be a way to perform validation also in such situations.

@ghost
Copy link

ghost commented Aug 25, 2015

Yup, i was already using the classic mongo query on this one. It's not really a problem since i use it in a behaviors where the values have already been validated.
Awesome to hear that the next release will tackle this issue. Astronomy just keep getting better and better ;)

@lukejagodzinski
Copy link
Member

Ok good to hear :). I'm trying to do my best :)

lukejagodzinski added a commit that referenced this issue Aug 29, 2015
@lukejagodzinski
Copy link
Member

I've implemented the push method for array fields. It works nicely with the new nested fields system. I closer and closer to releasing new version :). For now I'm closing this issue

@ghost
Copy link

ghost commented Aug 29, 2015

Awesome. Just so i understand properly, this new version will give me the possibility to push inside an array without having it in memory and without erasing the data already there ?

@lukejagodzinski
Copy link
Member

No, it just introduces push() method that adds element to the array. And later you can same the doc using doc.save(). You have to have it in the memory. The feature are you talking about is in another issue and it's named "direct database access and validation" or something like that :). I don't remember. However, the feature are you talking about will also appear :)

@ghost
Copy link

ghost commented Aug 30, 2015

Ok, i guess i'll see directly on the documentation to fully get everything this new version will bring ;)

lukejagodzinski added a commit that referenced this issue Sep 16, 2015
lukejagodzinski added a commit that referenced this issue Sep 27, 2015
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

3 participants