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

Breaks on 3.1 to 4.0 but nothing in readme to indicate what they are #93

Closed
dkebler opened this issue Jan 27, 2019 · 7 comments
Closed

Comments

@dkebler
Copy link

dkebler commented Jan 27, 2019

I tried updating from 3.1 to 4 and no surprise it broke my application. When I roll back all is fine.

The readme does not mention anything about what the breaking changes are which makes it time consuming to track them down. Just even a few comments in the readme would have been helpful.

Can you please indicate where it is likely I've have to make changes. Looks like the options have changed? Looks like you pass the model as a key value instead of the model itself? Anything else?

// Initializes the `circuits` service on path `/circuits`
const createService = require('feathers-nedb')
const createModel = require('../../helpers/nedb.model')
const hooks = require('./circuits.hooks')

module.exports = function () {
  const app = this
  const Model = createModel(app, 'circuits')
  const paginate = app.get('paginate')

  const options = {
    name: 'circuits',
    Model,
    events: ['changeRequest', 'changeComplete' ],  // custom event
    paginate
  }

  // Initialize our service with any options it requires
  app.use('/circuits', createService(options))

  // Get our initialized service so that we can register hooks and filters
  const service = app.service('circuits')

  service.hooks(hooks)
}
@dkebler
Copy link
Author

dkebler commented Jan 27, 2019

yea changed that to Model: Model and it's still breaking.

@dkebler
Copy link
Author

dkebler commented Jan 27, 2019

looks like hook.service.Model is undefined and yet my console.log below shows it's there under options. What up with that this extra key layer .options?

{ options:
   { events: [],
     paginate: { default: 100, max: 100 },
     multi: false,
     id: '_id',
     name: 'circuits',
     Model:
      Datastore {
        inMemoryOnly: false,
        autoload: true,
        timestampData: false,
        filename:
         '/mnt/238/nas/david/hacking/active-dev-repos/light2/database/circuits.db',
        compareStrings: undefined,
        persistence: [Persistence],
        executor: [Executor],
        indexes: [Object],
        ttlIndexes: {},
        schemas: [Object] } },

doesn't work.

// Sends back corresponding attached schemas when get when id 'schema' is used with get from client
module.exports = function () {
  return function(hook) {
    if (hook.id === 'schemas') {
      console.log('in get schemas', hook.service)
      // setting hook result will send schemas to client
      hook.result = hook.service.Model.schemas
      // console.log('getting schema', hook.result)
    }
    return hook
  }
}

works now with .options inserted

// Sends back corresponding attached schemas when get when id 'schema' is used with get from client
module.exports = function () {
  return function(hook) {
    if (hook.id === 'schemas') {
      console.log('in get schemas', hook.service.options.Model.schemas)
      // setting hook result will send schemas to client
      hook.result = hook.service.options.Model.schemas
      // console.log('getting schema', hook.result)
    }
    return hook
  }
}

So I found it myself so you can close this issue but...

can you please make a few comments in the readme for the benefit of others about this change to the object structure (and why?).

@daffl
Copy link
Member

daffl commented Jan 27, 2019

That's a good point actually, even the automatically generated Changelog didn't turn out to be very illuminating. You can find the migration information in crow.docs.feathersjs.com/migrating.html#database-adapters.

However, it shouldn't break anything in the initialization so more information about your setup and the error message would be helpful.

@dkebler
Copy link
Author

dkebler commented Jan 28, 2019

The issue is illustrated above. First to the model object I attach a schema object that holds a schema for records in that model. It was the easiest most obvious place to attach that for later use (schema goes with collection it is used against).

const NeDB = require('nedb')
// const path = require('path')

module.exports = function (app, service) {
// TODO find application root and save to app object for user here instead of __dirname
  // with this file in /helpers directory
  // console.log(__dirname +  '/../' + app.get('nedb').schemaPath + '/' + service + '.schemas.js')
  const schemas = require(__dirname +  '/../' + app.get('nedb').schemaPath + '/' + service + '.schemas.js')
  const dbPath = app.get('nedb').path
  const Model = new NeDB({
    filename: `${dbPath}/${service}.db`,
    autoload: true
  })
  Model.schemas = schemas

  if(app.get('nedb').autoCompact) {
    Model.persistence.setAutocompactionInterval(app.get('nedb').autoCompactInterval)
  }
  // console.log(Model.schemas.name)
  return Model
}

Later the client can issue a get with a custom id "schema" and the corresponding hook on the server (the code above) will return the schema rather than any record from the collection/service. The client uses that schema to make custom forms from vai framework (vuejs/quasar) that the service records then populate.

Also I have an init hook that needs access to the schema for record initialzation with default values from the schema.

const  getschemas = require('../../hooks/getschemas.js')
//const  init = require('../../hooks/init.js');
// const pDebounce = require('p-debounce')

const init = function () {
  // The hook function itself is returned.
  return function(hook) {
    // const { method, type, data, service } = hook;
    const { data, service } = hook
    // console.log('nedb model from hook in circuits.hooks.js', service.options.Model)
    const  schema = service.options.Model.schemas
    // console.log('hook log\n', data, schema)
    // validate name unique
    for (let field in schema) {
      // console.log('field, schema', field, schema[field])
      if (!data[field]) {
        data[field] = schema[field].default  // try to initize on type instead of default
        // console.log(field, data[field])
      }
    }
    data.on = false  // maintain circuit state
    //console.log('doc to be written', hook.data);
    return hook
  }
}

Basically anywhere where I needed access to the schema attached to the model the extra .options key was introduced that broke it.

What was the point of adding that key anyway? Was that to support the hookless call??

As is usually the case accessing a comlicated object directly is problematic. So the service object should include a getter/setter for the Model. That way one could mess with the structure of the service and still let users get a handle to the model. If you did that I'd refactor my code and then it won't break again in the future when the service object structure is altered cause you'll amend the getter/setter accordingly .

You should probably add getter/setter for any part of the service object that end users might have reasonable reason to access/amend

@daffl
Copy link
Member

daffl commented Jan 28, 2019

service.Model is still available. It is a read-only property that internally returns service.options.Model. If you want to change the model later you have to set service.options.Model.

@dkebler
Copy link
Author

dkebler commented Jan 28, 2019

I amended Model only where it was created not via the service so no issue there.

It was later when accessing it inside a hook the issue comes.

I'm not sure I agree with your comment that service.Model is available (everywhere) as a getter for service.options.Model otherwise my in my hooks (hook.service.Model) would not have broken. As you see above I had to use hook.service.options.Model to gain read access to the Model while in the hook.

So this then is maybe the issue. service.Model getter not available in hooks

@daffl
Copy link
Member

daffl commented Jan 28, 2019

I can't reproduce service.Model not being available in a hook but it's also true that the service.Model getter should not exist. service.options has been added to all database adapters to unify how options are passed and stored. If you always use service.options.Model going forward you should not see any further issues.

@dkebler dkebler closed this as completed Jan 29, 2019
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