HHH-16830 - apply filters to 'find()' method

[gregoriopalama]

https://hibernate.atlassian.net/browse/HHH-16830

[beikov]

This will have to wait until https://github.com/hibernate/hibernate-orm/pull/7735 is merged. Also, @sebersole mentioned that he dislikes the annotation member name. Let's wait what he proposes as alternative.

[hamid646m]

This will have to wait until #7735 is merged. Also, @sebersole mentioned that he dislikes the annotation member name. Let's wait what he proposes as alternative.

@beikov i believe that PR has been merge now

[gregoriopalama]

@sebersole following your comments on #7735 , should we use the same approach on XSD here? What about the annotation member name? I can update this accordly.

[sebersole]

What about the annotation member name?

My concern about the annotation attribute name is that I believe this actually affects operations other than just find. Which is fine as it is really the intent of this request - always apply the filter. But I think the name is therefore misleading.

I initially thought #alwaysApply but I think that it confusing with the concept of auto applied.

So long story short, I don't have a better suggestion at the moment. I just don't think #appliedOnFind is a good name.

[sebersole]

As I don't have a better suggestion right now, one option would be to mark the member as @Incubating for 6.5 and move on coming back to the naming later.

Though obviously that's a last resort.

[sebersole]

@beikov suggested #applyToLoadByKey which I think is reasonable.

[sebersole]

Actually after thinking about this some more and discussing within the team, I think we might want something slightly different here. Though I think the PR as-is is very close and you can easily make it match that.

What we'd prefer to see is :

1. By default apply filters as they were in 5.x, which I think your PR is already doing
2. Allow filters (via @FilterDef) to opt in to applying the filter to top level find operations. The implication here being fetching of to-one associations should never be filtered. I believe this is something we are missing here, right? "top level" here can/should include natural-id loading, multi id loading and multi natural-id loading.
3. Work on https://hibernate.atlassian.net/browse/HHH-17117 to allow deeper application of "filters".

So given this limited scope, I guess @FilterDef#applyOnFind is a good name.

I'd also like to see explicit tests that ensure fetching to-one associations does not apply the filters.

[beikov]

After todays team discussion, we concluded that we prefer the applyToLoadByKey member name. We decided that it is best to always apply filters on finds, regardless of whether it is for an association. To protect you from accidently destroying your own data (i.e. FKs set to null), we decided that it is best to apply the same technique that we use for @NotFound, which is to also fetch the FK of an association. This allows us to throw a NotFoundException if a filter would filter out the target row for an association.

We need a test that asserts this behavior though i.e. setup data that would filter out the association target row and assert that the exception is thrown.

[beikov]

Could you please rename the annotation member and do a rebase @gregoriopalama ?

I'll take care of implementing the part about

apply the same technique that we use for @NotFound

[hibernate-github-bot[bot]]

Thanks for your pull request!

This pull request does not follow the contribution rules. Could you have a look?

❌ All commit messages should start with a JIRA issue key matching pattern HHH-\d+     ↳ Offending commits: [d7bd4813585fc12d7bb6dcd28ea8c51195b11059, a99f3dfc2c21c8c0810a9dc0411e5c0ea4cf1afc, 4271c5adae73c6a999af5989dfb32051a9bbaf42, d38e348bc9df1fd49ae33f9f85dac08511002d62, d1ae79b381b9453c8eb5191ba613745b508537b6, c6b804e781106a759a279259e018210abf79427e, b620dcf14d9d23d333850e786843a28d701a9590, e677bc8a455e2ff3b58ee2240ed5875a2fdc3148, 01f55df182401f41d201de08c91a38fb639c72af, 927b27bc43a376216b0102c91f6f6f32ee6767b6, bf5d81aba532c71a2b351b5e78752f8ca4497b8a, 33f4fac6f532e749cd2dbc39f2fdf5ffc342b53e, dcb8a9644136188d2f430f277b3a3ce568a3771e, e73bed021cca788f2ae180c1c20d5f65c2167d78, 5273bae395f3cf78282bfce6532312daa31796dc, 9e79c963f0c856ba84f1ddc1e3489802f766ad6b, 883a61a911d67ed659a248d47d5a041b63f8befa, bd1196b7e11233b12bfa4decfe3e98e5f6b2d0f8, 6113a1a477d0d5155659d0c88ff38c6866fdc793, 01f6f9577db774f30b1321c9e1388e030f9752ed, c4fba2cad812f1734b0ac6dc2aaefce636477d83, cc00a7aedf6ffe4c4721f4a9efbd3b3eb35b334e, b2e98839a004d6d09dcfe462365b158e6b3b16a1, ddd656cab0aef57c6d80ca5a36546b67b812d23c, a0d6f98c3ecd8bb62a8eb7e15b0f822400ac0a7f, 003278a69153d7a07b4989172ee3b1d31248562a, 606c578390bff3f32916225630e1e630e7719276, ce94d37ebb35c4bc77a4cad35cf6a7984c71ea1b, 9e7281cadd3474ba1341043e4ed1d8035a88f636, 038e893c4844104d54d8115fb9a500b1139d96e0, b9d9c6f60372b61e192b37b58a7874a7c0938503, 3fc6592733358b1af699b6da6a9a3890fea04b63, 7db7e7ee53bd1a6257a8fc880684bdda487e3b09, 8b24e7ba3817d9527c1106d5694bb455a473d7f3, 84d10ee2c77e3b80009537049f75d8440c9731f9, 7bb000d1a567f78b42da9d63ce151e394e8a2139, 949721f3754bb091fb7f617ddffee27723719f1c, fc9aa329e5abac37c1f4485be157886b4104bd49, 0b592ea1b54c9d2b926a734956c7894fb9768db3, 56bfa854dab1bf69bcf5c4c64c3ecb9528e0e779, 2a3ff8a0c70e2bd8a6c1c53a4d967cbb1f1bae3a, cda943b56518943bacb0d9437a5da3d62ed18b02, d82c8ee9b888d79b6fb4148ab87fdb67aafa283c, 3a9536d4ee116f85dee22de8ab20aa41289d19c2, bc127629ca010a866d019ffa7e5203ec863b1760, 978919622a47c2da25408e69f087b557c8903186, 9e0c6941145341f5bdff3f2b9cf98430da1c7e93, 4193d33d5e03414b9499bfb8325c67522635ee78, 8d0fa635610881d01250f64c42b4538fff2fb7a9, 067ce8f8d1b5a3d2ed92ef45ad0cb2053cdb350f, 8a76bebb91c39f5d3cf9b11efafce69f830035a9, 26958ced75718b18feb20f8650b6815750704f15, 61172a0103d0d8deb6913268a56f668208190ac8, c251bcb694329cd69f1c29bc5bfd6906e431372b, a41edb6bcea33d2c47c61742f6da92ea9591b355, ab531556784df88b41790f800865cea33f81728c, 3ac52efc15bb18423d897fd64947e1181ed882dc, 5418552958800120f81de637db5c59aed6008ba1, 46450a5a3d137e538c60291d41312704a6fc15d5, 0fad5eecb1a185db67cdaf21045130e6e4f5f813, 762b574d0e51eab122ad3357329bdd00b44676ce, 2c771aecbce0d7752db51d087153c0c83e3d72e3, 5606130b6501f78b4d9b75a07122d0a7eda51d47, 7a96aa4f5b3947267e99461a13373dcaaff538a8, 1d9cfd2fed7519854d2f70050d2d20ba7ed369d9, ef2825aca7dc62f5481f1e7c24b1861a2e31771b, 84698285d5a6ed06e2f084f52967c5fc49ede8fb, 7b3ee5c6933d29f99b40c8a669bdbc6c941b3414, 80f9c82fedc1339ec49a89380982e93878c63b93, 6e1648695c959f69a61aea4b205c918980834b4b, 488c86861a0b1cc4ee808913253da02438b48c9d, c12aa4f21113fa4bdd4d2800eab636d031e537fb, d33f76cdcd2d7ae13c4b4793c412e68a820cdb4e, 3145dac9807da0fc8af5e912e575bdca6ae60728, c4e80fdcffd7530a072a990e11c77e3edb07bf46, d4b8ae863355f61ee2e13516fd954206148bf430, 6bfa73fd159ef252989d3ccee7bcc158c92c6968, f744ef88499a8d221757894951c33da1de9ea42f, 3ebdeae92da6336a99a3eec6ccb2bbb9eb39560a, e4e4d3f7dd24860d312c26e9c08c04c31c118244, f2b56d7335b7b2fb56bb1303e65fe7b664e02889, f61b94efa04d17b6d9e6d85225685b85d58c2322, e7795fece6454e23b5d787167cb5f514e4e7b357, 2f2fc81a200f69dcc8341582081116ee558eefb9, 91fbcfd4224a1aa869dcfa6cb1bcc0411534be59, e95605a2fd0f602f61e722dbcdbc357b2990ae70, 545f0fa0f864d4a43e5b5fa99e66f984ae3070e7, 104ad21671c7e8a25d4152df2c23764258c55931, b55b6bec7f169f9a4025d6713dcc9e8e8f952509, 8ef4fecf87c8d6d85f436846ea6e6cb147f434cf, ea983a0e3ade2606fc40023851e4bd45ccd80352, b00ece9f1c92336b78060695701f30c6d858f53a, f46e4ebcdd3bd4d5bf6b7eced66756b7ad87a66b, 70c46525195e0eda2ab471cffe78df308bacd5be, 16ec9b28324b6e66f3e5391cfcb23a2143895f11, 0d3ec1b05251860a697a8a2b951a3dde936f6ac6, ce2197e8d40750b579241c035d8b1265590cdbb0, b77e7f044eb7560ecb89d3c13c81ec5d924cd6ed, 0ef1c45cba39bbbcb7d64632b0e302f30eb69be2] ❌ The PR title or body should list the keys of all JIRA issues mentioned in the commits     ↳ Issue keys mentioned in commits but missing from the PR title or body: [HHH-17779, HHH-17726, HHH-17772, HHH-17778, HHH-17795, HHH-17769, HHH-17800, HHH-17791, HHH-17763, HHH-17804, HHH-17789, HHH-17776, HHH-17794, HHH-17805, HHH-17797, HHH-17808, HHH-17807, HHH-17813, HHH-17638, HHH-17269, HHH-17806, HHH-17817, HHH-9110, HHH-17831, HHH-17833, HHH-17825, HHH-17836, HHH-17133, HHH-16985, HHH-17773, HHH-15587, HHH-17711, HHH-17823, HHH-17777, HHH-17671, HHH-17839, HHH-14810, HHH-17600, HHH-17848, HHH-17579, HHH-17596, HHH-17850, HHH-17851, HHH-17853, HHH-17830, HHH-17743, HHH-17859, HHH-17858, HHH-17854, HHH-9482, HHH-17868, HHH-17073, HHH-17568, HHH-14968, HHH-17759, HHH-17472, HHH-17874, HHH-17873, HHH-17875, HHH-17860, HHH-17864, HHH-17878, HHH-17872, HHH-17801, HHH-17867, HHH-17891, HHH-17876, HHH-17895, HHH-10619, HHH-14080, HHH-17862, HHH-17897, HHH-17736, HHH-17429, HHH-17824, HHH-17871, HHH-12202, HHH-17493, HHH-17884, HHH-17904, HHH-17906, HHH-17803]

› This message was automatically generated.

[gregoriopalama]

@beikov not sure everything went smooth with the rebase. Before I create issues, may I ask you if I made something wrong? I see complaints from the bot on commits name, but those are from the rebase.

[beikov]

The rebase didn't work as you can see on the UI based on the message about conflicting files.

You need to git fetch from the "upstream" repository https://github.com/hibernate/hibernate-orm first, then rebase your branch against that upstream main branch. After that, force push to your branch.

[kevinm416]

Hi @gregoriopalama, not sure what went wrong with your rebase, but this is what I would do to try to resolve the situation:

# Not sure if you need to add a remote, but I did after checking out your repo. 
# If you have an upstream when you run `git remote` then you can skip this.
$ git remote add upstream [email protected]:hibernate/hibernate-orm.git
$ git fetch upstream

# check out new branch based on hibernate's up-to-date main and switch to it
$ git checkout -b main-2 upstream/main
# cherry pick the commits from your branch
$ git cherry-pick 1fd3dcccca14bb890387eafef22034680d366fda
$ git cherry-pick 98d21c937c84db21d9d5660ac2afeb6a208abead
# Resolve conflicts with upstream (there are some small ones), then commit
# Delete your original feature branch
$ git branch -D HHH-16830
# Rename the the new branch to match the original branch name
$ git branch -m HHH-16830
# Force push your feature branch now that it's up-to-date with upstream/main
$ git push -f HHH-16830

I did these steps locally, but obviously can't attempt pushing the results.

[kingan379]

Any updates on this MR? It is a big security problem holding many teams back.

[kevinm416]

This is a good starting point for how to work around this issue: https://github.com/M-Devloo/Spring-boot-auth0-discriminator-multitenancy/pull/3. I found the CriteriaBuilder approach used in that PR too slow, but it was very helpful to see how to set up a global base repository.

[karmann-dm]

Hey folks, any update on the issue?

[beikov]

Superseded by https://github.com/hibernate/hibernate-orm/pull/8463