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

Cannot get Virtual Attributes to show up in any api response #117

Closed
kaizenseed opened this issue Sep 18, 2020 · 8 comments
Closed

Cannot get Virtual Attributes to show up in any api response #117

kaizenseed opened this issue Sep 18, 2020 · 8 comments

Comments

@kaizenseed
Copy link

kaizenseed commented Sep 18, 2020

Hello & thanks for the awesome package 👍

I'm seeing an issue that on the surface looks to be similar to #45 except I am not sure which version of objection, feathers-objection resulted in the virtual attributes no longer being part of the api responses.

My (simplified) model class :

class Subscriber extends Model {

  static get tableName() {
    return 'subscribers';
  }
  static get jsonSchema() {
    return {
      type: 'object',
      required: ['email'],

      properties: {
        id: { type: 'integer' },
        email: { type: 'string', format: 'email' }, // validating this here via pattern did not work
        name: { type: 'string' },
        twitterHandle: { type: 'string', pattern: "^(@[a-zA-Z0-9_]{1,16})?$" },
        // pattern : must begin with @ and be followed by alphanumeric string or _  and only 15 chanracters allowed
        about: { type: 'string' },
        approvedAt: { type: 'string', format: 'date-time' },
      }
    };
  }

  static get virtualAttributes() {
    return ['approved'];
  }

  get approved() {
    return this.approvedAt !== null
  }
}

the service is the standard service generated by feathers cli. I've tried overriding the model level parsing functions but they don't get hit in a simple get call at all

api response to a get call is as follows (notes is a column in the db but not defined in JsonSchema)
{ "id": 1, "email": "blah@gmail.com", "name": "Blah", "twitterHandle": "", "about": "", "notes": null }

Any pointers to what I may be doing wrong would help

From my package.json file
"objection": "^2.2.3", "feathers-objection": "^5.8.1",

@dekelev
Copy link
Member

dekelev commented Sep 18, 2020 via email

@kaizenseed
Copy link
Author

Makes sense. I'll try tracing out where it broke by downgrading the objection package and update here with results.

@kaizenseed
Copy link
Author

kaizenseed commented Sep 19, 2020

Huh. Weird.
I was able to see the virtual fields in the response after I disabled an after hook in feathers

 context => iff(
        (isProvider('external') && context.service.options.secretFields),
        discard(...context.service.options.secretFields))(context),

$formatJson also gets called if i comment the above hook 🤔

the secretFields for this service includes additional meta fields like updatedAt approvedAt etc

@dekelev
Copy link
Member

dekelev commented Sep 20, 2020

@kaizenseed Did you comment out code in the feathers NPM module?

@kaizenseed
Copy link
Author

kaizenseed commented Sep 20, 2020

ah no sorry for the confusion.. I'll show all my code again

subscriber.model.js

/* eslint-disable no-console */

// subscribers-model.js
//
// See http://knexjs.org/
// for more of what you can do here via knex.
// See https://vincit.github.io/objection.js/#models
// for more of what you can do here via objection.
const { BaseModel } = require('./base.model')
//model schema defined in knex migrations
class Subscriber extends Model {
  static get tableName() {
    return 'subscribers';
  }
  static get jsonSchema() {
    return {
      type: 'object',
      required: ['email'],

      properties: {
        id: { type: 'integer' },
        email: { type: 'string', format: 'email' }, // validating this here via pattern did not work
        name: { type: 'string' },
        twitterHandle: { type: 'string', pattern: "^(@[a-zA-Z0-9_]{1,16})?$" },
        // pattern : must begin with @ and be followed by alphanumeric string or _  and only 15 chanracters allowed
        about: { type: 'string' },
        createdAt: { type: 'string', format: 'date-time' },
        updatedAt: { type: 'string', format: 'date-time' },
        approvedAt: { type: 'string', format: 'date-time' },
        signupEmailStatus: 'string'
      }
    };
  }

  $beforeInsert() {
    this.createdAt = this.updatedAt = new Date().toISOString();
  }

  $beforeUpdate() {
    this.updatedAt = new Date().toISOString();
  }

  static get metaFields() {
    // these fields are not editable by external providers
    // and so should be discarded before create, update & patch calls
    return ['createdAt', 'updatedAt', 'approvedAt', 'signupEmailStatus']
  }

  static get secretFields() {
    // these fields should be scrubbed from the final response returned to external providers
    // typically is a superset including metaFields
    return ['approvedAt', 'signupEmailStatus', 'id', 'createdAt', 'updatedAt']
  }

  static get createNeedsUnique() {
    return ['email']
  }

  static get virtualAttributes() {
    return ['approved'];
  }

  get approved() {
    return this.approvedAt !== null
  }

  // This object defines the relations to other models.
  static get relationMappings() { }
}
// table creation if not exists is now managed by knex migrations
// so just run some basic tests and throw an error reminding user to run the knex migrations
module.exports = function (app) {
  const db = app.get('knex');
  const tableName = Subscriber.tableName;
  try {
    db.schema.hasTable(tableName)
      .then(function (exists) {
        if (!exists) {
          throw `Table ${tableName} does not exist. Ensure that you have run all migrations.\nto run all migrations to latest use 'knex migrate:latest --env production'`;
        }
        else {
          // add additional schema checks here corresponding to future migrations
          // db.schema.hasColumn(tableName, columnName)
        }
      })
      .catch(err => console.error(err));
  } catch (err) {
    console.error(`exception thrown : ${err}`)
  }
  // eslint-disable-line no-console

  return Subscriber;
};

The subscriber service

const configureOptions = (app, model) => {
  const options = {
    Model: model,
    paginate: app.get('paginate'),
    metaFields: model.metaFields ? model.metaFields : [],
    secretFields: model.secretFields ? model.secretFields : [],
    createNeedsUnique: model.createNeedsUnique ? model.createNeedsUnique : [],
    virtualAttributes: model.virtualAttributes ? model.virtualAttributes : []
  };
  return options;
}

module.exports = function (app) {
  const options = configureOptions(app, createModel(app));

  // Initialize our service with any options it requires
  app.use('/subscribers', new Subscribers(options, app));

  // Get our initialized service so that we can register hooks
  const service = app.service('subscribers');

  service.hooks(hooks);
};

The subscriber hooks & subscriber class is fairly standard

app.hooks.js

// Application hooks that run for every service
const { iff, isProvider, discard } = require('feathers-hooks-common')

const discardMetaFields = (context) => iff(
  (isProvider('external') && context.service.options.metaFields),
  discard(...context.service.options.metaFields)
)(context)

const returnRecordsIfUniqueExists = async context => {
  if (!context.service.options || !context.service.options.createNeedsUnique) return context;
  const uniqueList = context.service.options.createNeedsUnique;
  if (uniqueList.length === 0) return context;
  const queryOrArray = []
  for (let field of uniqueList) {
    if (context.data.hasOwnProperty(field)) {
      const obj = {}
      obj[field] = context.data[field]
      queryOrArray.push(obj)
    }
  }
  const records = await context.app.service(context.path).find({
    query: {
      $or: queryOrArray
    }
  });
  if (records.total >= 1) {
    // if yes, return the associated record.
    // * we do not patch the original record with any new data provided
    // Update the result (the message)
    context.result = records.total == 1 ? records.data[0] : records.data
  }
  // Returning will resolve the promise with the `context` object
  return context;
}

const { debugHook } = require('./utils/customhooks/common.hooks')
module.exports = {
  before: {
    all: [],
    find: [],
    get: [],
    create: [discardMetaFields, returnRecordsIfUniqueExists],
    update: [discardMetaFields],
    patch: [discardMetaFields],
    remove: []
  },

  after: {
    all: [
      context => iff(
        (isProvider('external') && context.service.options.secretFields),
        discard(...context.service.options.secretFields))(context),   
 ],
    find: [],
    get: [],
    create: [],
    update: [],
    patch: [],
    remove: []
  },

  error: {
    all: [
      context => console.error(`Error in '${context.path}' service method '${context.method}'`, context.error.stack)
    ],
    find: [],
    get: [],
    create: [],
    update: [],
    patch: [],
    remove: []
  }
};

So no modification of anything at the package level, just added application level hooks to handle removing respective metaFields from input, and hiding secretFields from output for all services in my app. The metaFields discarding doesn't affect anything but apparently the hook dealing with discarding the secret fields from the response in the after.all somehow stops the model's $formatJson(json) from being called. If I remove this particular hook, I get all fields in the response, including the secretFields AND the virtual fields.

@kaizenseed
Copy link
Author

kaizenseed commented Sep 26, 2020

Hi I was finally able to look at this some more and 99% sure this doesn't have anything to do with feathers-objection.. there's some magic happening with the discard part of the hook. Leaving that here for anyone who faces similar issues and closing the issue. Thanks for the library again 👍

@dekelev
Copy link
Member

dekelev commented Sep 26, 2020

Thank you @kaizenseed 👍

@kaizenseed
Copy link
Author

kaizenseed commented Sep 26, 2020

additional info for documentation + want to get it confirmed that my thought process is correct

  • so I'm assuming the virtualfields get added to the response at whichever step we have a toJson functionality which is basically like JSON.stringify(context.result)
  • discard hook deleted the 'secret' fields from the subscriber object in the context.result
  • I'm assuming this would stop objection/feathers-objection from then identifying the object as a Subscriber model and doing the conversion? 🤔

This is based on that I got it to work by replacing the after hook as follows

const cloneDeep = require('lodash/cloneDeep');
const {
  protect
} = require('@feathersjs/authentication-local').hooks;
const copyDispatchToResult = async context => {
  if (context.dispatch && context.result) {
    context.result = cloneDeep(context.dispatch)
  }
  return context
}
.
.
.
all:[
context => iff(
        (isProvider('external') && context.service.options.secretFields),
        // discard(...context.service.options.secretFields),
        protect(...context.service.options.secretFields),
        copyDispatchToResult,
      )(context)
]

the protect hook apparently does something very similar to discard except it operates on context.dispatch by default rather than result and it internally calls the toJSON equivalent explicitly before deleting the fields (this discard doesn't do)

Thus I'm assuming even though the object is no longer recognized by objection as the Subscriber model, toJson has already been called and hence all is right with the response 😛

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