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

Question: Embedded Documents #140

Open
dev-inigmas opened this issue Jul 10, 2014 · 24 comments
Open

Question: Embedded Documents #140

dev-inigmas opened this issue Jul 10, 2014 · 24 comments

Comments

@dev-inigmas
Copy link

What is the correct (and simplest) means of notifying EPF about Embedded Documents? Earlier, I thought that I had figured it out; but now, I'm running into the follow issue:

SO.Foo = Ep.Model.extend
  ### Properties ###
  name:        Ep.attr 'string'
  description: Ep.attr 'string'

  ### Relationships ###
  bars: Ep.hasMany 'bar'

  ### Computed ###
  summary: ( ->
    @get('bars').mapProperty('name').join(', ')
  ).property('[email protected]')

SO.FooSerializer = SO.ApplicationSerializer.extend
  properties:
    bars: embedded: 'always'

SO.Bar = Ep.Model.extend
  ### Properties ###
  name: Ep.attr 'string'

  ## Parent Document
  foo: Ep.belongsTo 'foo', inverse: 'bars'

SO.FooIndexRoute = Ember.Route.extend
  model: -> @session.query 'foo'

When going to the "foo.index" route, I get the following:

Error while processing route: foo.index Cannot read property 'isEqual' of undefined TypeError: Cannot read property 'isEqual' of undefined
    at Ep.ModelArray.Ember.ArrayProxy.extend.removeObject (http://bebop.local:9494/dist/deps.js:73508:34)
    at apply (http://bebop.local:9494/dist/deps.js:31391:27)
    at superWrapper (http://bebop.local:9494/dist/deps.js:30969:15)
    at null.<anonymous> (http://bebop.local:9494/dist/deps.js:74282:50)
    at Ep.Model.reopen.suspendRelationshipObservers (http://bebop.local:9494/dist/deps.js:73210:37)
    at Ep.InverseManager.Ember.Object.extend._removeFromInverse (http://bebop.local:9494/dist/deps.js:74280:23)
    at Ep.InverseManager.Ember.Object.extend.unregisterRelationship (http://bebop.local:9494/dist/deps.js:74261:26)
    at null.<anonymous> (http://bebop.local:9494/dist/deps.js:74234:30)
    at Mixin.create.forEach (http://bebop.local:9494/dist/deps.js:39783:20)
    at null.<anonymous> (http://bebop.local:9494/dist/deps.js:74233:37) 

Anyone know what's going on?

Fyi - things seem to load fine when I don't specify an inverse. But then, EPF tries to POST/PUT the embedded document(s) separately.

Thanks.

@ghempton
Copy link
Contributor

Are you using recent build from master?

@ghempton
Copy link
Contributor

You also shouldn't need to specify an inverse in this case

@ghempton
Copy link
Contributor

Also, what does SO.ApplicationSerializer look like?

@dev-inigmas
Copy link
Author

I just checked out EPF and built it from the master branch yesterday.

I agree that I shouldn't have to specify an explicit inverse in this case. On another model, doing that seemed to prevent EPF from sending invalid POSTs/PUTs.

Here's the code:

SO.ApplicationSerializer = Ep.ActiveModelSerializer.extend()

@ghempton
Copy link
Contributor

Can you paste an example payload?

On Thu, Jul 10, 2014 at 5:23 PM, David Patrick [email protected]
wrote:

I just checked out EPF and built it from the master branch yesterday.

I agree that I shouldn't have to specify an explicit inverse in this
case. On another model, doing that seemed to prevent EPF from sending
invalid POSTs/PUTs.

Here's the code:

SO.ApplicationSerializer = Ep.ActiveModelSerializer.extend()


Reply to this email directly or view it on GitHub
#140 (comment).

Gordon L. Hempton
http://codebrief.com
360.460.8098

@dev-inigmas
Copy link
Author

Which payload? A GET response? The POST/PUT requests?

@ghempton
Copy link
Contributor

GET response, or both?

On Thu, Jul 10, 2014 at 5:24 PM, David Patrick [email protected]
wrote:

Which payload? A GET response? The POST/PUT requests?


Reply to this email directly or view it on GitHub
#140 (comment).

Gordon L. Hempton
http://codebrief.com
360.460.8098

@dev-inigmas
Copy link
Author

Seems like the explicit inverse doesn't break "findById" requests.... I'll get the responses now.

@dev-inigmas
Copy link
Author

Here's a response for a single item:

{
  "race": {
    "doc_meta": {
      "created":1405024824835,
      "updated":1405029317207,
      "created_by":"53bd768f970000004057a9f4"
    },
    "id":"53befa38970000ff7257aa18",
    "name":"qwe",
    "description":"qwe",
    "abilities": [{
      "ability":"qwe",
      "description":"qwe"
    }],
    "module_id":"53bef0b0970000771f57aa17"
  }
}

@ghempton
Copy link
Contributor

Hmm, I think this might be caused by the nested ability not having and id.
Can you try passing a fixed id property just to check if this is the case?
There isn't a real reason why EPF would require an id in an embedded model,
but it could be a bug.

On Thu, Jul 10, 2014 at 5:31 PM, David Patrick [email protected]
wrote:

Here's a response for a single item:

{
"race": {
"doc_meta": {
"created":1405024824835,
"updated":1405029317207,
"created_by":"53bd768f970000004057a9f4"
},
"id":"53befa38970000ff7257aa18",
"name":"qwe",
"description":"qwe",
"abilities": [{
"ability":"qwe",
"description":"qwe"
}],
"module_id":"53bef0b0970000771f57aa17"
}
}


Reply to this email directly or view it on GitHub
#140 (comment).

Gordon L. Hempton
http://codebrief.com
360.460.8098

@dev-inigmas
Copy link
Author

Here is a very truncated response from the server during a @session.query 'race':

{
  "races": [{
    "doc_meta": {
      "created":1377814485629,
      "updated":1394501556095,
      "created_by":"50c923c6e4b0add373778525"
    },
    "id":"50ea5298e4b0c6e36078e0aa",
    "name":"Android",
    "description":"Androids...",
    "abilities": [{
      "ability":"Recharge",
      "description":"During..."
    }, {
      "ability":"Unnatural",
      "description":"Arcane powers..."
    }],
    "module_id":"50c9277de4b0add373778527"
  }]
}

@dev-inigmas
Copy link
Author

Putting a static "id" of 1 in there made the exceptions go away. But the abilities also went away.

@ghempton
Copy link
Contributor

Ah so I think the issue is that the abilities have a belongsTo on them.
When this is the case it epf currently expects the ability to have the
parent id specified in their payload, e.g. race_id: '50ea5298e4b0c6e36078e0aa'.
This definitely seems like an area that could be improved.

On Thu, Jul 10, 2014 at 5:56 PM, David Patrick [email protected]
wrote:

Putting a static "id" of 1 in there made the exceptions go away. But the
abilities also went away.


Reply to this email directly or view it on GitHub
#140 (comment).

Gordon L. Hempton
http://codebrief.com
360.460.8098

@ghempton
Copy link
Contributor

Have you tried removing the belongsTo entirely from the ability?a

On Thu, Jul 10, 2014 at 5:56 PM, David Patrick [email protected]
wrote:

Putting a static "id" of 1 in there made the exceptions go away. But the
abilities also went away.


Reply to this email directly or view it on GitHub
#140 (comment).

Gordon L. Hempton
http://codebrief.com
360.460.8098

@dev-inigmas
Copy link
Author

Yeah; then I get extra POST/PUT from EPF.

@dev-inigmas
Copy link
Author

At some point, I saw a comment in the EPF that said something about embedded documents requiring inverses... I could try to find it again.

@dev-inigmas
Copy link
Author

Particularly, line 49 of epf/lib/rest/embedded_manager.js

@ghempton
Copy link
Contributor

I might have made that unnecessary, but I think the safest solution for the short term would be to include the parent ID in the embedded model. Hopefully this can be cleaned up soon.

@dev-inigmas
Copy link
Author

I probably could do that.... But I have whole lot of Model types that make use of this pattern. Changing them all would be very time-consuming. Is there a slightly less safe tweak I could try out in the JavaScript, before resorting to that approach?

@ghempton
Copy link
Contributor

You could create a custom HasMany serializer and a custom Model
serializer base class.

Inside the HasMany serializer you could override the deserializeEmbedded
to pass in some extra data through the options parameter to automatically
set the parent in the model serializer.

On Thu, Jul 10, 2014 at 6:11 PM, David Patrick [email protected]
wrote:

I probably could do that.... But I have whole lot of Model types that
make use of this pattern. Changing them all would be very time-consuming.
Is there a slightly less safe tweak I could try out in the JavaScript,
before resorting to that approach?


Reply to this email directly or view it on GitHub
#140 (comment).

Gordon L. Hempton
http://codebrief.com
360.460.8098

@dev-inigmas
Copy link
Author

Sounds good..... How do I register the custom HasMany serializer with EPF again? I know how to do it for a model. A HasMany override seems like it might be more explicit though.

@ghempton
Copy link
Contributor

Unlike ember data, there is no distinction between a transform and a serializer (they are both just objects that have a serialize and a deserialize method) and they are looked up the same way. Looks like you aren't using EAK or ember-cli, so in your case you would just follow the right naming convention:

App.HasManySerializer = Ep.HasManySerializer.extend({...})

Cheers

@dev-inigmas
Copy link
Author

(edited to provide configurable implementation and example Model definitions)

So, this seems to work:

SO.ApplicationSerializer = Ep.ActiveModelSerializer.extend
  extractProperty: (model, hash, name, type, opts) ->
    if opts?.embedded && hash.id? && hash[name]?.length > 0
      hash[name].forEach (m) =>
        m[@_prop(name)]=hash.id
    @_super(model, hash, name, type, opts)

  _prop: (name) ->
    (@configFor(name)?.inverse || @get('typeKey'))+'_id'

And here are my Models:

# Represents a sentient creature type; like Humans, Elves, or Androids
SO.Race = Ep.Model.extend
  ### Properties ###
  name:        Ep.attr 'string'
  description: Ep.attr 'string'

  ### Relationships ###
  abilities: Ep.hasMany 'race_ability'

# An effect the Race has on a character
SO.RaceAbility = Ep.Model.extend
  ### Properties ###
  ability:     Ep.attr 'string'
  description: Ep.attr 'string'
  script:      Ep.attr 'string'

  ## Parent Document
  race: Ep.belongsTo 'race', inverse: 'abilities'

SO.RaceSerializer = SO.ApplicationSerializer.extend
  properties:
    abilities:
      embedded: 'always'
      inverse:  'race'

I wasn't sure what extra data the HasManySerializer had access to that it would be able to pass in a suitable option to the ModelSerializer... So I ended up not overriding it.

See any downsides to this approach?

@ghempton
Copy link
Contributor

Hmm at a glance it all looks like it could work out. Keep me posted as to how this plays out.

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

2 participants