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 about usage performance #161

Closed
lonre opened this issue Aug 13, 2021 · 14 comments
Closed

Question about usage performance #161

lonre opened this issue Aug 13, 2021 · 14 comments
Labels
question Further information is requested

Comments

@lonre
Copy link
Contributor

lonre commented Aug 13, 2021

builder.constraintOnTarget(param -> {
  Document doc = documentDao.findById(param.getId());
  if (isNull(doc)) {
    return false;
  }
  return true;
}, "id", "not_exists", "id not exists");

builder.constraintOnTarget(param -> {
  Document doc = documentDao.findById(param.getId());
  if (!param.getOwnerId().equals(doc.getOwnerId())) {
    return false;
  }
  return true;
}, "ownerId", "no_permission", "no permission");

this validator will execute documentDao.findById twice, is it possible to make the documentDao.findById execute only once?

Thanks

@making
Copy link
Owner

making commented Aug 13, 2021

Unfortunately, YAVI does not support such cases.
As a workaround, you can define a validator for am.ik.yavi.fn.Pair

Example

Validator<Pair<Document, Param>> validator = ValidatorBuilder.<Pair<Document, Param>>of()
		.constraintOnObject(Pair::first, "id", c -> c.notNull().message("{0} not exists"))
		.constraintOnTarget(pair -> pair.first() == null || Objects.equals(pair.first().getOwnerId(), pair.second().getOwnerId()),
				"ownerId", "no_permission", "no permission")
		.build();


validator.validate(new Pair<>(documentDao.findById(param.getId()), param));

@making making added the question Further information is requested label Aug 13, 2021
@lonre
Copy link
Contributor Author

lonre commented Aug 13, 2021

Unfortunately, YAVI does not support such cases.
As a workaround, you can define a validator for am.ik.yavi.fn.Pair

Example

Validator<Pair<Document, Param>> validator = ValidatorBuilder.<Pair<Document, Param>>of()
		.constraintOnObject(Pair::first, "id", c -> c.notNull().message("{0} not exists"))
		.constraintOnTarget(pair -> pair.first() == null || Objects.equals(pair.first().getOwnerId(), pair.second().getOwnerId()),
				"ownerId", "no_permission", "no permission")
		.build();


validator.validate(new Pair<>(documentDao.findById(param.getId()), param));

@making Thanks for your help.

but the full validator buider is

builder._string(Param::getFielda, "fielda", a -> a.notNull().notBlank());
builder._string(Param::getFieldb, "fieldb", a -> a.notNull().notBlank());
builder._long(Param::getId, "id", Constraint::notNull);
builder.constraintOnTarget(param -> {
  Document doc = documentDao.findById(param.getId());
  if (isNull(doc)) {
    return false;
  }
  return true;
}, "id", "not_exists", "id not exists");

builder.constraintOnTarget(param -> {
  Document doc = documentDao.findById(param.getId());
  if (!param.getOwnerId().equals(doc.getOwnerId())) {
    return false;
  }
  return true;
}, "ownerId", "no_permission", "no permission");

If there is some workaround please?

@making
Copy link
Owner

making commented Aug 13, 2021

@lonre

Validator<Pair<Document, Param>> validator = new ValidatorBuilder<Pair<Document, Param>>("" /* message key separator */)
		.nest(Pair::second, "", b -> b
				.constraint(Param::getId, "id", c -> c.notNull())
				.constraint(Param::getFieldA, "fieldA", c -> c.notNull().notBlank())
				.constraint(Param::getFieldB, "fieldB", c -> c.notNull().notBlank()))
		.constraintOnObject(Pair::first, "id", c -> c.notNull().message("{0} not exists"))
		.constraintOnTarget(pair -> pair.first() == null || Objects.equals(pair.first().getOwnerId(), pair.second().getOwnerId()),
				"ownerId", "no_permission", "no permission")
		.build();

@lonre
Copy link
Contributor Author

lonre commented Aug 13, 2021

@lonre

Validator<Pair<Document, Param>> validator = new ValidatorBuilder<Pair<Document, Param>>("" /* message key separator */)
		.nest(Pair::second, "", b -> b
				.constraint(Param::getId, "id", c -> c.notNull())
				.constraint(Param::getFieldA, "fieldA", c -> c.notNull().notBlank())
				.constraint(Param::getFieldB, "fieldB", c -> c.notNull().notBlank()))
		.constraintOnObject(Pair::first, "id", c -> c.notNull().message("{0} not exists"))
		.constraintOnTarget(pair -> pair.first() == null || Objects.equals(pair.first().getOwnerId(), pair.second().getOwnerId()),
				"ownerId", "no_permission", "no permission")
		.build();

ok I see, Thanks for your help again @making

@making making closed this as completed Aug 13, 2021
@lonre
Copy link
Contributor Author

lonre commented Aug 26, 2021

Hi @making

this is commons use cases in our projects, do you any idea or any plan to support this case

Thanks

@making
Copy link
Owner

making commented Aug 26, 2021

It's hard to achieve it under the current design.

I would use caching like Spring Framework provides
https://spring.io/guides/gs/caching/

@lonre
Copy link
Contributor Author

lonre commented Aug 26, 2021

It's hard to achieve it under the current design.

I would use caching like Spring Framework provides
https://spring.io/guides/gs/caching/

Hi, Currently I am considering to fork yavi, and have a dirty hack. Extends ViolatedValue with specified violation details,

and hack the validate method, if ViolatedValue has details, use that, otherwise use original logic 😂

@making

lonre added a commit to lonre/yavi that referenced this issue Aug 26, 2021
@making
Copy link
Owner

making commented Aug 26, 2021

Can you add test cases? I'm not sure what your commits want to achieve

@lonre
Copy link
Contributor Author

lonre commented Aug 27, 2021

Can you add test cases? I'm not sure what your commits want to achieve

Hi @making

test added

lonre@ccf8970

@making
Copy link
Owner

making commented Aug 30, 2021

Sorry, but I can't accept your proposal.
Again, It's hard to achieve it under the current design.

It may require big changes to the code base

@lonre
Copy link
Contributor Author

lonre commented Aug 30, 2021

Sorry, but I can't accept your proposal.
Again, It's hard to achieve it under the current design.

It may require big changes to the code base

Hi @making

This make sense, and thanks for your time.

It is just dirty changes, and for my own usage.

@atkawa7
Copy link

atkawa7 commented Jun 17, 2022

@making Thanks for this library. I have been using yavi java-poet and jsonschema to create validators and this issue keeps on propping up. For example I have custom validators to validate phone numbers and jwt etc. However I have to parse the values twice one which I would discard and another which I would use because there is no caching on the return value.

@making
Copy link
Owner

making commented Jun 17, 2022

@atkawa7 can you open a new issue? This is closed.
And please share a minimal reproducible working example to get the better context.

@atkawa7
Copy link

atkawa7 commented Jun 17, 2022

Thanks @making I have created a new ticket copying this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants