jagi / meteor-astronomy

Model layer for Meteor
https://atmospherejs.com/jagi/astronomy
MIT License
605 stars 66 forks source link

Behavior when using Astronomy Class on clients. #476

Closed axelvaindal closed 8 years ago

axelvaindal commented 8 years ago

Hi,

Considering this publication:

import { User } from './Users.js';

Meteor.publish('userData', function()
{
    let user = User.findOne({_id: this.userId});
   // Options is set with expected value for published fields

    return Meteor.users.find(this.userId, options);
}

I have different results when using Meteor.user() and User.findOne({_id: userId}) on the client. The Meteor.user() actually displays only the fields which were published previously, as expected, but the User.findOne() version displays every field, even tough they are not in the database client-side.

I did db.users.find() to see my results in my client database, so I'm 100% sure my data are correct.

Is that a normal behavior (ie Client class fetch every field on client no matter the actual data on client) or is there a known bug ? @jagi

lukejagodzinski commented 8 years ago

If you defined some fields in a class then even if these fields are not published they will appear in the user object but they will be empty (undefined). It's normal behavior.

axelvaindal commented 8 years ago

I get that but still I have the value which are not undefined :/.

Meteor.user():

id:"oAyuztQTs6kRiH2Co"
emails: Array[1]
lastLoggedAt: Wed Aug 10 2016 17:53:44 GMT+0200 (Paris, Madrid (heure d’été))
location:""
profile:Object
quote:""
status:1
username:"test"
__proto__: Object

User Class Find:

firstName:""
fullName: "undefined undefined"
gender:0
language:"en"
lastLoggedAt:Wed Aug 10 2016 17:53:44 GMT+0200 (Paris, Madrid (heure d’été))
lastName:""
location:""
metaData:Object
phones:Array[0]
profile:Object
quote:""
settings:Object
state:1
status:1
updatedAt: undefined
username:"test"

Gender should be undefined and is not, should'nt it ? Same for location ?

I print here User class for example:

/*
|--------------------------------------------------------------------------
| User Class and Collection
|--------------------------------------------------------------------------
|
| This file defines the User Class and import the User collection.
| User Collection is generated inside the AccountsPassword package.
*/

import { Class } from 'meteor/jagi:astronomy';
import { Enum } from 'meteor/jagi:astronomy';

export const UserStatus = Enum.create(
{
    name: 'UserStatus',
    identifiers: ['OFFLINE', 'ONLINE', 'WORKING', 'BUSY', 'AWAY']
});

export const UserGender = Enum.create(
{
    name: 'UserGender',
    identifiers: ['MALE', 'FEMALE']
});

export const UserState = Enum.create(
{
    name: 'UserState',
    identifiers: ['ACTIVE', 'INACTIVE', 'ASLEEP', 'DEACTIVATED', 'DELETED', 'BANNED']
});

export const UserFieldVisibility = Enum.create(
{
    name: 'Visibility',
    identifiers: ['PUBLIC', 'PRIVATE', 'HIDDEN', 'INVISIBLE']
});

export const User = Class.create(
{
    name: "User",
    collection: Meteor.users,
    fields: 
    {
        lastLoggedAt:
        {
            type: Date,
            optional: true
        },
        username:
        {
            type: String,
            default: function() 
            {
                return "";
            }
        },
        profile:
        {
            type: Object,
            optional: true,
            default: function() 
            {
                return {};
            }
        },
        emails: 
        {
            type: [Object],
            default: function()
            {
                return [];
            },
        },
        status:
        {
            type: UserStatus,
            default: function() 
            {
                return UserStatus.OFFLINE;
            }
        },
        language:
        {
            type: String,
            default: function() 
            {
                return "en";
            }
        },
        firstName: 
        {
            type: String,
            optional: true,
            default: function() 
            {
                return "";
            }
        },
        lastName:
        {
            type: String,
            optional: true,
            default: function() 
            {
                return "";
            }
        },
        fullName:
        {
            type: String,
            optional: true,
            resolve(doc) 
            {
                return doc.firstName + ' ' + doc.lastName;
            },
        },
        birthDate:
        {
            type: Date,
            optional: true
        },
        age: 
        {
            type: Number,
            transient: true,
            optional: true
        },
        quote:
        {
            type: String,
            optional: true,
            default: function() 
            {
                return "";
            }
        },
        location:
        {
            type: String,
            optional: true,
            default: function() 
            {
                return "";
            }
        },
        gender:
        {
            type: UserGender,
            optional: true,
            default: function() 
            {
                return UserGender.MALE;
            }
        },
        phones: 
        {
            type: [String],
            optional: true,
            default: function() 
            {
                return [];
            }
        },
        settings:
        {
            type: Object,
            optional: true,
            default: function() 
            {
                return {};
            }
        },
        state:
        {
            type: UserState,
            optional: true,
            default: function() 
            {
                return UserState.INACTIVE;
            }
        },
        metaData:
        {
            type: Object,
            optional: true,
            default: function() 
            {
                return {};
            }
        },
        fieldVisibilities:
        {
            type: Object,
            default: function()
            {
                return {
                    createdAt: UserFieldVisibility.INVISIBLE,
                    updatedAt: UserFieldVisibility.INVISIBLE,
                    deleted: UserFieldVisibility.INVISIBLE,
                    deletedAt: UserFieldVisibility.INVISIBLE,
                    lastLoggedAt: UserFieldVisibility.PUBLIC,
                    username: UserFieldVisibility.PUBLIC,
                    emails: UserFieldVisibility.HIDDEN,
                    status: UserFieldVisibility.PUBLIC,
                    language: UserFieldVisibility.HIDDEN,
                    firstName: UserFieldVisibility.PRIVATE,
                    lastName: UserFieldVisibility.PRIVATE,
                    fullName: UserFieldVisibility.PRIVATE,
                    birthDate: UserFieldVisibility.PRIVATE,
                    age: UserFieldVisibility.PRIVATE,
                    quote: UserFieldVisibility.PUBLIC,
                    location: UserFieldVisibility.PUBLIC,
                    gender: UserFieldVisibility.PRIVATE,
                    phones: UserFieldVisibility.HIDDEN,
                    settings: UserFieldVisibility.HIDDEN,
                    state: UserFieldVisibility.INVISIBLE,
                    metaData: UserFieldVisibility.INVISIBLE,
                    fieldVisibilities: UserFieldVisibility.INVISIBLE,
                }
            }
        }
    },
    methods:
    {
        getStatusName()
        {
            return UserStatus.getIdentifier(this.status);
        },
        isOffline()
        {
            return Utils.isEqual(this.status, UserStatus.OFFLINE);
        },
        isOnline()
        {
            return Utils.isEqual(this.status, UserStatus.ONLINE);
        },
        isWorking()
        {
            return Utils.isEqual(this.status, UserStatus.WORKING);
        },
        isBusy()
        {   
            return Utils.isEqual(this.status, UserStatus.BUSY);
        },
        isAway()
        {
            return Utils.isEqual(this.status, UserStatus.AWAY);
        },
        isMale()
        {
            return Utils.isEqual(this.status, UserGender.MALE);
        },
        isFemale()
        {
            return Utils.isEqual(this.status, UserGender.MALE);
        },
        isActive()
        {
            return Utils.isEqual(this.status, UserState.ACTIVE);
        },
        isInactive()
        {
            return Utils.isEqual(this.status, UserState.INACTIVE);
        },
        isAsleep()
        {
            return Utils.isEqual(this.status, UserState.ASLEEP);
        },
        isDeactivated()
        {
            return Utils.isEqual(this.status, UserState.DEACTIVATED);
        },
        isDeleted()
        {
            return Utils.isEqual(this.status, UserState.DELETED);
        },
        isBanned()
        {
            return Utils.isEqual(this.status, UserState.BANNED);
        },
        getPublishedFields(visibility)
        {
            let fields = this.getFields(visibility);
            let publishedFields = {};

            if (Utils.isArray(fields))
            {
                if (!Utils.isEmpty(fields))
                {
                    for (f of fields)
                    {
                        publishedFields[f] = 1;
                    }
                }
            }

            return publishedFields;
        },
        getFields(visibility)
        {
            let fields = [];

            if (Utils.isArray(visibility))
            {
                if (!Utils.isEmpty(visibility))
                {
                    for (v of visibility)
                    {
                        for (f in this.fieldVisibilities)
                        {
                            if (this.fieldVisibilities[f] === v)
                            {
                                fields.push(f);
                            }
                        }
                    }
                }

            }

            return fields;
        },
    },
    events: 
    {
        afterInit(e) 
        {
            const doc = e.currentTarget;

            if (!Utils.isFalsy(doc.birthDate)) 
            {
                let diff = Date.now() - birthDate.getTime();
                doc.age = Math.abs((new Date(diff)).getUTCFullYear() - 1970);
            }
        }
    },
    behaviors: 
    {
        timestamp: 
        {
            hasCreatedField: true,
            createdFieldName: 'createdAt',
            hasUpdatedField: true,
            updatedFieldName: 'updatedAt'
        },
        softremove: 
        {
            removedFieldName: 'deleted',
            hasRemovedAtField: true,
            removedAtFieldName: 'deletedAt'
        }
    }
});

Are the default value messing it ? EDIT: Yes they are, I assumed default value would only be used when insert when data is not provided. Is their a way to define default value inside your object and still not get them on client ?

Sorry for the annoyance, and thanks for your answer,

@jagi

lukejagodzinski commented 8 years ago

You can define some fields to be only available on the server, everything is in the documentation.

if (Meteor.isServer) {
  User.extend({
    fields: {
      serverOnlyField: {
        type: String,
        default() { return 'SOME VALUE'; }
      }
    }
  });
}
axelvaindal commented 8 years ago

I was more expecting a way of getting undefined if data is not published, no matter the default value set on the object. I would like the default value to trigger only on insert, but this is maybe the role of your beforeInsert event and not the default(). If it's an anti-pattern about your way of doing things, I can do default value on onCreateUser hook or beforeInsert, I just found pretty useful to use your default() method on the User class considering it seemed self explicit :)

lukejagodzinski commented 8 years ago

Yes you can do exactly what you said. I don't actually know how often is the default property used and in what cases. It seemed to be a correct way of doing it. Maybe this behavior should be changed to only be triggered on insert but that would probably break code of many applications that are already using Astronomy.

axelvaindal commented 8 years ago

Yes, this would probably be hard breaking for many applications already using Astronomy. And after further investigation, it seems not to be a so much needed behavior in my current application, except for user data which have special visibility and thus many way of being published. In consequence, I will just go for onCreateUser hook to set default value for this particular use case.

Thanks for your time and help :).

lukejagodzinski commented 8 years ago

No problem :)

panphora commented 7 years ago

I just ran into this myself and it was super confusing! Would be great if it was mentioned in the docs somewhere. Perhaps a note under "Fetching documents" that mentions that if you have defaults on a property, this default will show up as the value for the field even if you're not publishing it.

lukejagodzinski commented 7 years ago

@panphora Yes I will this information to the docs. In deed it might be confusing that default values are set also on document fetch. I wonder if there is a way of solving this without breaking compatibility. Maybe some config option. I agree that it is bad design issue. It definitely should only use default values on insert or update, however it should not set undefined fields with a default value.

lukejagodzinski commented 7 years ago

@panphora Ok I've implemented a way of doing it without breaking compatibility. In Astronomy 2.3.0 you will be able to do:

Post.find(selector, {
  defaults: false
});

So empty fields in documents fetched from database won't be filled with defaults values. So it will work nicely with the fields options.

Post.find(selector, {
  defaults: false,
  fields: {
    title: 0 // Default value won't be used
  }
});

However, you have to remember even though value of the field will be undefined the property for a field in a document will be created, so previous example will return.

{
  otherField: 'value',
  number: 4,
  title: undefined
}

Of course, I will add a proper info in the documentation.

panphora commented 7 years ago

@jagi Awesome, thank you so much! This will help a lot.