Keys array with self-reference doesn't work

[ulitiy]

Due to asynchronous requests, replaceKeyFields is called too late, after we have already sent the referenced object to the server. If we make a breakpoint and wait for response, the referenced object is sent to the server correctly.

It would be great if it was possible to have self-reference in keys. For example: tag: parent_tag_id -> tag so we could find all dirty highest level parents, push them, then on success in pushItem recursively push descendants.

[alekseykulikov]

Sounds good! We should check parent_tag and if it's dirty push them to the server than push changed item on success, right?

[ulitiy]

Смотри, допустим я в оффлайне создал теги a, b, c, при этом a.parent_tag=b, b.parent_tag=c, они находятся в одной коллекции. Потом делаю push, он сразу делает pushItem для всех элементов, получается что a и b получают parent_tag_id=0 вместо правильных b.id, c.id соответственно.

Я предлагаю: в pushItem мы находим предков высшего уровня (тех, у которых parent_tag_id не локальный), то есть, в данном случае – с, потом on success b, потом on success a – рекурсивно.

Но можно и как ты предлагаешь - брать все подряд элементы и смотреть, является ли parent_tag_id локальным, и в таком случае сначала делать pushItem для предка, а собственно элемент – on success. Но в этом случае придётся сделать так, чтобы элементы не сохранялись дважды - неправильно и правильно.

Причем таких полей может быть много, например, БД людей с разными типами отношений между людьми. Таким образом будет например поле mother_id, father_id, которые ссылаются в ту же коллекцию и т.д.

Надеюсь, понятно изъясняюсь. Я не очень глубоко разбирался в исходниках, поэтому не знаю, какой подход будет лучше.

[alekseykulikov]

Ok, thank you! I understand your example ;) My and your method are in a similar way.

[ulitiy]

This simple patch is for the my case of 1 field. Also it's not clear how to sync several collections at one time with a server if we have asynchronous requests, so the second collection also can have dirty fields when we make push() for it, am I right? So we need success linking of all dirty models? To make dirty models to wait for the setting of the key fields. And to make sure, that all the "dirty fields" are set before we push it to the server.

initialize:  ->
  @storage = new Offline.Storage('blocks', this, keys: {parent_id: this})

  @storage.sync.pushItem= (item,options={}) ->
    return if /^\w{8}-\w{4}-\w{4}/.test(item.get("parent_id")) && !options.skipCheck?
    @storage.replaceKeyFields(item, 'server')
    localId = item.id
    delete item.attributes.id
    [method, item.id] = if item.get('sid') is 'new' then ['create', null] else ['update', item.attributes.sid]
    this.ajax method, item, success: (response, status, xhr) =>
      item.set(sid: response.id) if method is 'create'
      item.save {dirty: false}, {local: true}
      @pushItem(i, skipCheck: true) for i in item.children()

[alekseykulikov]

Thank you @ulitiy for your patch! I will think about this problem and try to fix it as soon as possible.

[ulitiy]

You can't universally solve this task without double saving, because there is at least a problem with cycling referencing (ex. a.brother_id=b.id, b.brother_id=a.id). So the simplest way is to save all models, then save second time all models, that have updated key fields due to replaceKeyFields.

[ulitiy]

Monkey patch with double saving if it's necessary

Offline.Storage.prototype.replaceKeyFields= (item, method) ->
  if Offline.onLine()
    item = item.attributes if item.attributes
    for field, collection of @keys
      replacedField = item[field]
      if !/^\w{8}-\w{4}-\w{4}/.test(replacedField) or method isnt 'local'
        newValue = if method is 'local'
          wrapper = new Offline.Collection(collection)
          wrapper.get(replacedField)?.id
        else
          ref_item=collection.get(replacedField)
          if ref_item?
            item.dirty=true #added
            ref_item.get('sid')
        item[field] = newValue unless _.isUndefined(newValue)
  return item

Offline.Sync.prototype.push= (options)->
  window.inPush=0 #added
  this.pushItem(item,options) for item in @collection.dirty()
  this.flushItem(sid) for sid in @storage.destroyIds.values

Offline.Sync.prototype.pushItem= (item,options) ->
  @storage.replaceKeyFields(item, 'server')
  localId = item.id
  delete item.attributes.id
  [method, item.id] = if item.get('sid') is 'new' then ['create', null] else ['update', item.attributes.sid]
  window.inPush++ #added
  this.ajax method, item,
    success: (response, status, xhr) =>
      item.set(sid: response.id) if method is 'create'
      item.save {dirty: false}, {local: true}
      window.inPush-- #added
      if inPush==0 #added
        @push inner_success: options?.success #added, to make chain collection pushes: push tags-on success->push posts
      options?.inner_success?() #added

But the weakness is to ensure, we have only one sync run, because of global inPush variable

[alekseykulikov]

@ulitiy thanks for your activity ;) You can pull request your code ;) And If you using this library we be able to discuss it in skype. My name is alekseys.kulikov

[alekseykulikov]

@ulitiy I added a simple guide for contributors https://github.com/Ask11/backbone.offline#how-to-contribute It will be great if you will put your patch as pull request.