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.

Sign up for GitHub

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

Jump to bottom

Set which entities classes should never be cached, even indirectly #2744

Merged
merged 19 commits into from
Mar 20, 2022
Merged

Set which entities classes should never be cached, even indirectly #2744

merged 19 commits into from
Mar 20, 2022

Conversation

gokhanabatay
Copy link
Contributor

@gokhanabatay gokhanabatay commented May 2, 2021 •
edited
Loading

Hi,
When Second level cache enabled "never" cached entities should not interact with UpdateTimestampsCache(#2735), for backward compability as discussed here new Cache Strategy added.

After release I could implement FluentNHibernate

maca88
maca88 previously approved these changes May 30, 2021
View reviewed changes
@gokhanabatay gokhanabatay mentioned this pull request Jun 2, 2021
Closed
@gokhanabatay
Copy link
Contributor Author

@maca88 as community will you add this new feature to next minor release?

Also when merge of this pull request, #2130 pull request could be declined

@gokhanabatay gokhanabatay mentioned this pull request Jun 2, 2021
Closed
@maca88
Copy link
Contributor

maca88 commented Jun 3, 2021

@maca88 as community will you add this new feature to next minor release?

In case other NHibernate members will also approve it, it will be most likely included in the next minor release.

@gokhanabatay
Copy link
Contributor Author

gokhanabatay commented Jun 16, 2021 •
edited
Loading

@maca88 as community will you add this new feature to next minor release?

In case other NHibernate members will also approve it, it will be most likely included in the next minor release.

@fredericDelaporte @hazzik @bahusoid @gliljas could you please review these feature?

@gokhanabatay
Copy link
Contributor Author

@maca88 no members other than you paid attention to this pr? How can we continue? Is it normal procedure?

@hazzik
Copy link
Member

hazzik commented Jun 21, 2021

I’ve paid attention, but I don’t have enough expertise to properly review that fast. It would take longer for me to do proper review.

fredericDelaporte
fredericDelaporte reviewed Jul 2, 2021
View reviewed changes
Copy link
Member

@fredericDelaporte fredericDelaporte left a comment •
edited by hazzik
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with this PR overall. Some minor adjustments are needed.

src/NHibernate/Action/BulkOperationCleanupAction.cs Show resolved Hide resolved
src/NHibernate/Action/BulkOperationCleanupAction.cs Outdated Show resolved Hide resolved
src/NHibernate/Action/BulkOperationCleanupAction.cs Outdated Show resolved Hide resolved
src/NHibernate/Action/BulkOperationCleanupAction.cs Outdated Show resolved Hide resolved
src/NHibernate/Engine/ISessionFactoryImplementor.cs Outdated Show resolved Hide resolved
src/NHibernate/Loader/Custom/CustomLoader.cs Outdated Show resolved Hide resolved
src/NHibernate/Loader/Custom/CustomLoader.cs Outdated Show resolved Hide resolved
src/NHibernate/Loader/Custom/CustomLoader.cs Outdated Show resolved Hide resolved
src/NHibernate/Loader/Loader.cs Outdated Show resolved Hide resolved
src/NHibernate/Loader/Loader.cs Outdated Show resolved Hide resolved
@gokhanabatay
Copy link
Contributor Author

Any updates on this issue, how can I help?

gokhan.abatay@gmail.com and others added 10 commits January 16, 2022 01:35
@maca88
code review fixes
9a4616c
@maca88
Reset to commit IExecutable
f221947
@maca88
ICacheableCollectionPersister added, cache strategy can also be set f…
644a9ea 
…or each collection
@maca88
Never cached entity cached query exception improved
a514c59
@maca88
Code refactoring and add a shared interface for entity and collection…
7136c0b 
… persisters
@maca88
Never cached entities/collections cannot be used in a cacheable query…
0636107 
… default true it could be disabled via query.throw_never_cached option
@maca88
add xml configuration validation
0007404
@maca88
Fix typos
dd7419a
@maca88 @fredericDelaporte
Update src/NHibernate/Action/BulkOperationCleanupAction.cs
52d88e2 
Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Action/BulkOperationCleanupAction.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Action/BulkOperationCleanupAction.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Engine/ISessionFactoryImplementor.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Loader/Custom/CustomLoader.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Impl/SessionFactoryImpl.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Impl/SessionFactoryImpl.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Action/BulkOperationCleanupAction.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Loader/Custom/CustomLoader.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Loader/Custom/CustomLoader.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Loader/Loader.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Loader/Loader.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Action/BulkOperationCleanupAction.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Action/BulkOperationCleanupAction.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Action/CollectionAction.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Action/EntityAction.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Action/ICacheableExecutable.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Cache/CacheFactory.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Cfg/Environment.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Cfg/MappingSchema/Hbm.generated.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Engine/ActionQueue.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Hql/Ast/ANTLR/Tree/DotNode.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Loader/Custom/CustomLoader.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>

Update src/NHibernate/Loader/Custom/CustomLoader.cs

Co-authored-by: Frédéric Delaporte <12201973+fredericDelaporte@users.noreply.github.com>
@maca88
Add SupportsQueryCache extension methods
92f158d
@maca88
Copy link
Contributor

maca88 commented Jan 16, 2022

Squashed some commits and rebased.

@fredericDelaporte
Document the never cache strategy
fe5fb09 
And regenerate the Hbm file.
fredericDelaporte
fredericDelaporte approved these changes Jan 23, 2022
View reviewed changes
src/NHibernate/Cfg/MappingSchema/Hbm.generated.cs
[System.CodeDom.Compiler.GeneratedCodeAttribute("HbmXsd", "5.0.0-Alpha1")]
[System.CodeDom.Compiler.GeneratedCodeAttribute("HbmXsd", "5.4.0-dev")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Granted, regenerating it is a bit noisy.

src/NHibernate/Cfg/MappingSchema/Hbm.generated.cs
Comment on lines -985 to +989

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it does still avoid introducing formatting discrepancies which would could other noises on subsequent regeneration.

src/NHibernate/Cfg/MappingSchema/Hbm.generated.cs
Comment on lines +430 to +433

/// <remarks/>
[System.Xml.Serialization.XmlEnumAttribute("never")]
Never,
Copy link
Member

@fredericDelaporte fredericDelaporte Jan 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the "by hand" writing of the code was also lacking the (almost) empty blank line. (I write "almost" because the generator does put spaces in it.)

@gokhanabatay
Copy link
Contributor Author

We are going through a very busy time, I see mentions today sory for that @fredericDelaporte.
When will we add this feature to "fluent nhibernate" package? Can we use fluent nhibernate from myget nuget store?

@fredericDelaporte
Copy link
Member

There is no MyGet currently for FluentNHibernate.

@fredericDelaporte
Copy link
Member

It seems this PR is awaiting reviews from @bahusoid and @gliljas. If you want a functional view of what have been done, have a look at the changes in the documentation.

bahusoid
bahusoid reviewed Feb 4, 2022
View reviewed changes
Copy link
Member

@bahusoid bahusoid left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fredericDelaporte
I don't think you should wait for requested reviews unless it was self requested.

It's already been reviewed and approved by you and maca88. Seems quite enough for me. Plus I don't have much time - better spend it on some other issues

@fredericDelaporte fredericDelaporte added this to the next minor milestone Mar 13, 2022
@fredericDelaporte fredericDelaporte changed the title Never Cached Entities should not interact with UpdateTimestampsCache Set which entities classes should never be cached, even indirectly Mar 13, 2022
@fredericDelaporte fredericDelaporte merged commit 8b74774 into nhibernate:master Mar 20, 2022
@fredericDelaporte fredericDelaporte mentioned this pull request Mar 20, 2022
Closed
@gokhanabatay gokhanabatay mentioned this pull request Mar 23, 2022
Closed
@fredericDelaporte fredericDelaporte mentioned this pull request Sep 13, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hazzik hazzik hazzik left review comments

@maca88 maca88 maca88 left review comments

@bahusoid bahusoid bahusoid left review comments

@fredericDelaporte fredericDelaporte fredericDelaporte approved these changes

@gliljas gliljas Awaiting requested review from gliljas

Assignees
No one assigned
Labels
c: L2 Cache p: Minor r: Fixed t: New Feature
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@gokhanabatay @maca88 @hazzik @fredericDelaporte @gliljas @bahusoid