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

getModelSchemaRef is not returning a property from a model that extends an entity #4388

Closed
rodrigomf24 opened this issue Jan 8, 2020 · 11 comments
Assignees

Comments

@rodrigomf24
Copy link

Steps to reproduce

  • create an lb4 app following the docs guide to add JWT authentication
  • secure all endpoints
  • run the app
  • test the create user and login endpoints through the explorer endpoint, it should work
  • add a new model to relate with the user model
  • run the lb4 relation command to create a hasMany relationship(user has many > new model)
  • migrate
  • run app
  • test the create user and login endpoints through the explorer endpoint, it should work(but it doesn't)

Current Behavior

  • The explorer API is not showing the password property from the NewUserRequest model passed to getModelSchemaRef for the request body of the users post endpoint. When the create endpoint handler is tested through the explorer API and the password attribute is passed the additionalProperties error is raised, when it is not passed it raises a validation error:
"name": "UnprocessableEntityError",
"message": "password must be minimum 8 characters"

Expected Behavior

  • The explorer API should show the password field as required and as part of the schema.

Link to reproduction sandbox

Additional information

This is the model passed to getModelSchemaRef:

@model()
export class NewUserRequest extends User {
  @property({
    type: 'string',
    required: true,
  })
  password: string;
}

This is the User model:

@model({
  settings: {
    indexes: {
      uniqueEmail: {
        keys: {
          email: 1,
        },
        options: {
          unique: true,
        },
      },
    },
  },
})
export class User extends Entity {
  @property({
    type: 'number',
    id: true,
    generated: true,
  })
  id?: number;

  @property({
    type: 'string',
    required: true,
  })
  email: string;

  @property({
    type: 'string',
  })
  first_name?: string;

  @property({
    type: 'string',
  })
  last_name?: string;

  @hasOne(() => UserCredentials, {keyTo: 'user_id'})
  userCredentials: UserCredentials;

  @hasMany(() => Organization, {keyTo: 'user_id'})
  organizations: Organization[];

  constructor(data?: Partial<User>) {
    super(data);
  }
}

and this is the user repository

import {
  DefaultCrudRepository,
  repository,
  HasOneRepositoryFactory,
  HasManyRepositoryFactory,
} from '@loopback/repository';
import {User, UserCredentials, Organization} from '../models';
import {inject, Getter} from '@loopback/core';
import {UserCredentialsRepository} from './user-credentials.repository';
import {XDataSource} from '../datasources';
import {OrganizationRepository} from './organization.repository';

export type Credentials = {
  email: string;
  password: string;
};

export class UserRepository extends DefaultCrudRepository<
  User,
  typeof User.prototype.id
> {
  public readonly userCredentials: HasOneRepositoryFactory<
    UserCredentials,
    typeof User.prototype.id
  >;

  public readonly organizations: HasManyRepositoryFactory<
    Organization,
    typeof User.prototype.id
  >;

  constructor(
    @inject('datasources.x') dataSource: XDataSource,
    @repository.getter('UserCredentialsRepository')
    protected userCredentialsRepositoryGetter: Getter<
      UserCredentialsRepository
    >,
    @repository.getter('OrganizationRepository')
    protected organizationRepositoryGetter: Getter<OrganizationRepository>,
  ) {
    super(User, dataSource);
    this.organizations = this.createHasManyRepositoryFactoryFor(
      'organizations',
      organizationRepositoryGetter,
    );
    this.registerInclusionResolver(
      'organizations',
      this.organizations.inclusionResolver,
    );
    this.userCredentials = this.createHasOneRepositoryFactoryFor(
      'userCredentials',
      userCredentialsRepositoryGetter,
    );
  }

  async findCredentials(
    userId: typeof User.prototype.id,
  ): Promise<UserCredentials | undefined> {
    try {
      return await this.userCredentials(userId).get();
    } catch (err) {
      if (err.code === 'ENTITY_NOT_FOUND') {
        return undefined;
      }
      throw err;
    }
  }
}

This is the user controller post/create definition

@post('/users', {
    security: [],
    responses: {
      '200': {
        description: 'User',
        content: {
          'application/json': {
            schema: {
              'x-ts-type': User,
            },
          },
        },
      },
    },
  })
  async create(
    @requestBody({
      content: {
        'application/json': {
          schema: getModelSchemaRef(NewUserRequest, {
            title: 'NewUser',
            exclude: ['id'],
          }),
        },
      },
    })
    newUserRequest: NewUserRequest,
  ): Promise<User> {
    // ensure a valid email value and password value
    validateCredentials(_.pick(newUserRequest, ['email', 'password']));
...

Related Issues

See Reporting Issues for more tips on writing good issues

@rodrigomf24 rodrigomf24 added the bug label Jan 8, 2020
@dougal83
Copy link
Contributor

dougal83 commented Jan 8, 2020

Hey @rodrigomf24

Try comparing your implementation to the shopping example. If there is a problem with the documentation please point it out.

I'd be happy to take a look at your code if you can provide a repo replicating your issue(upload to git as public repository).

@ricky92
Copy link

ricky92 commented Jan 11, 2020

Looks like a duplicate of #3293 to me.
You are extending an already-defined model and thus are incurring in the same bug described there. I don't think it's related to anything wrong with the documentation.

@dougal83 dougal83 added developer-experience Issues affecting ease of use and overall experience of LB users and removed bug labels Jan 13, 2020
@rodrigomf24
Copy link
Author

@dougal83 here is the link to the repository https://github.com/rodrigomf24/medipharma-api, I will compare it today, but @ricky92's comment makes sense, I will try this solution:

{
  ownMetadataOnly: true,
}

@dougal83
Copy link
Contributor

Looking at the code, it is working as expected. Also typically we do not return a password due to it's sensitive nature.

Looking at the controller you are actually returning the User model, that by design excludes the password.

@post('/users')
async create( ... ): Promise<User>

If you want to return the password(not a good idea):

@post('/users', {
    responses: {
      '200': {
        description: 'NewUserWITHPASSWORDHASH!',
        content: {
          'application/json': {
            schema: getModelSchemaRef(NewUserRequest),
          },
        },
      },
    },
  })
  async create(
    @requestBody({
      content: {
        'application/json': {
          schema: getModelSchemaRef(NewUserRequest, {
            title: 'NewUser',
            exclude: ['id'],
          }),
        },
      },
    })
    newUserRequest: NewUserRequest,
  ): Promise<NewUserRequest> {
    // ensure a valid email value and password value
    validateCredentials(_.pick(newUserRequest, ['email', 'password']));

    // encrypt the password
    const password = await this.passwordHasher.hashPassword(
      newUserRequest.password,
    );

    try {
      // create the new user
      const savedUser = await this.userRepository.create(
        _.omit(newUserRequest, 'password'),
      );

      // set the password
      await this.userRepository
        .userCredentials(savedUser.id)
        .create({password});

      const newUser = new NewUserRequest(savedUser);
      newUser.password = password;

      return newUser;
    } catch (error) {
      ...
    }
  }

Hope that clears up what is going on here.

@dougal83
Copy link
Contributor

dougal83 commented Jan 13, 2020

Looks like a duplicate of #3293 to me.
You are extending an already-defined model and thus are incurring in the same bug described there. I don't think it's related to anything wrong with the documentation.

Thanks @ricky92. I think your issue is loosely related but this particular case is down to the controller returning a model that doesn't contain the include.

@dougal83 dougal83 removed the developer-experience Issues affecting ease of use and overall experience of LB users label Jan 13, 2020
@rodrigomf24
Copy link
Author

Looking at the code, it is working as expected. Also typically we do not return a password due to it's sensitive nature.

Looking at the controller you are actually returning the User model, that by design excludes the password.

@post('/users')
async create( ... ): Promise<User>

If you want to return the password(not a good idea):

@post('/users', {
    responses: {
      '200': {
        description: 'NewUserWITHPASSWORDHASH!',
        content: {
          'application/json': {
            schema: getModelSchemaRef(NewUserRequest),
          },
        },
      },
    },
  })
  async create(
    @requestBody({
      content: {
        'application/json': {
          schema: getModelSchemaRef(NewUserRequest, {
            title: 'NewUser',
            exclude: ['id'],
          }),
        },
      },
    })
    newUserRequest: NewUserRequest,
  ): Promise<NewUserRequest> {
    // ensure a valid email value and password value
    validateCredentials(_.pick(newUserRequest, ['email', 'password']));

    // encrypt the password
    const password = await this.passwordHasher.hashPassword(
      newUserRequest.password,
    );

    try {
      // create the new user
      const savedUser = await this.userRepository.create(
        _.omit(newUserRequest, 'password'),
      );

      // set the password
      await this.userRepository
        .userCredentials(savedUser.id)
        .create({password});

      const newUser = new NewUserRequest(savedUser);
      newUser.password = password;

      return newUser;
    } catch (error) {
      ...
    }
  }

Hope that clears up what is going on here.

Hi @dougal83, thanks for taking a look at the code. I am not trying to return the password, the problem I have is when I try to create a new user I want it to use the NewUserRequest schema to allow passing a password field, but for some reason it is not picking that one and it is using the extended one which is User.

@dougal83
Copy link
Contributor

@rodrigomf24 Oh right, I think I understand you now. I was over-complicating the issue in my head.

 const savedUser = await this.userRepository.create();

Your code is calling create on the userRepository that is using the User model. Try altering the user repository to use the NewUserRequest model and see if that works as you expect.

@dougal83
Copy link
Contributor

dougal83 commented Feb 1, 2020

Potentially related to #4547

@cycorld
Copy link

cycorld commented Feb 5, 2020

@dougal83 I agree with you. When I add a relation with User and use "includeRelations" option at responses, the password is disappear.
And when I call "getModelSchemaRef(User)" at another place, it also disappear.

@dougal83
Copy link
Contributor

dougal83 commented Feb 6, 2020

Looking further into this issue... I presume the main stumbling block may be around the requirement to register an inclusion resolver before it can be used. Please take a read of the documentation and see if it helps or if you could suggest an improvement?

I had a go at altering the loopback4 shopping example and found that the inclusion resolves if I add the following line after the relation is configured:

this.registerInclusionResolver('userCredentials', this.userCredentials.inclusionResolver);

To test I created an endpoint to consume the filter:

@get('/users', {
    responses: {
      '200': {
        description: 'Array of User model instances',
        content: {
          'application/json': {
            schema: {
              type: 'array',
              items: getModelSchemaRef(User, { includeRelations: true }),
            },
          },
        },
      },
    },
  })
  async find(
    @param.query.object('filter', getFilterSchemaFor(User))
    filter?: Filter<User>,
  ): Promise<User[]> {
    return this.userRepository.find(filter);
  }

Finally I queried via browser:

http://localhost:3000/users?filter[include][0][relation]=userCredentials

Does that give you a starting point? IMO it is most likely the inclusion registration being the issue.

@dougal83
Copy link
Contributor

Closing. Feel free to reopen as necessary.

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

4 participants