Skip to content

Add document loaded/addedForStorage interception methods#200

Merged
jeremydmiller merged 2 commits intoJasperFx:masterfrom
tim-cools:Add-document-loaded/addedForStorage-interception-methods
Mar 11, 2016
Merged

Add document loaded/addedForStorage interception methods#200
jeremydmiller merged 2 commits intoJasperFx:masterfrom
tim-cools:Add-document-loaded/addedForStorage-interception-methods

Conversation

@tim-cools
Copy link
Copy Markdown
Contributor

I added two methods to the IDocumentSessionListener to intercept when documents are loaded/stored.

void DocumentLoaded(object id, object document);
void DocumentAddedForStorage(object id, object document);

This interception is currently implemented in the default and DirtyChecking IdentityMap.

I was tempted to split the IDocumentSessionListener into a IDocumentSessionListener and IDocumentListener. The latter would contain the new methods. But I decided to go for the simplest implementation for now. Let me know what your thoughts are about this?

If you agree I can add this also to the documentation if you want.

@ekulakov
Copy link
Copy Markdown
Contributor

@tim-cools Maybe I'm wrong but I think you could write a decorator and reuse it for all IdentityMap's

    public class IdentityMapListenersDecorator : IIdentityMap
    {
        private readonly IIdentityMap _identityMap;
        private readonly IEnumerable<IListener> _listeners;

        public IdentityMapListenersDecorator(IIdentityMap identityMap, IEnumerable<IListener> listeners)
        {
            _identityMap = identityMap;
            _listeners = listeners;
        }

        public T Get<T>(object id, Func<FetchResult<T>> result) where T : class
        {
            var obj = _identityMap.Get(id, result);
            _listeners.Each(x => x.DocumentLoaded(id, obj));
            return obj;
        }

        ...
    }

@tim-cools
Copy link
Copy Markdown
Contributor Author

@ekulakov thanks for the feedback!This is however not possible because the listeners are only called when a document is loaded the first time and not when the document is returned from the cache.

tim-cools@306bb17#diff-99cac1569732f4ed1d27aceb9e8f52ebR64

A decorator is unaware of the internals of the decorated class. We might consider pulling out a generic base class for both IdentityMaps however. I will look into that later...

@jeremydmiller jeremydmiller added this to the v0.9 milestone Mar 11, 2016
public IdentityMap(ISerializer serializer, IEnumerable<IDocumentSessionListener> listeners)
{
_serializer = serializer;
_listeners = listeners?.Any() == true ? listeners : null;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@tim-cools I'd rather we used a "nullo" empty list here instead of having to remember to always use the "?" operator. Nit pick-y, I know.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no problem, will update it in a next pull request. it was a silly attempt of premature optimization...

@jeremydmiller
Copy link
Copy Markdown
Member

I've got a minor comment about using a nullo for the list of listeners, but other than that I think this looks good. The build passed, and I'm pulling this in now.

jeremydmiller added a commit that referenced this pull request Mar 11, 2016
…orage-interception-methods

Add document loaded/addedForStorage interception methods
@jeremydmiller jeremydmiller merged commit 1521c2f into JasperFx:master Mar 11, 2016
jeremydmiller added a commit that referenced this pull request Apr 30, 2026
Picks up the fix for CultureNotFoundException at DocumentStore..ctor on
hosts running with <InvariantGlobalization>true</InvariantGlobalization>
(JasperFx PR #200): the cold-start prefilter in
ProjectionGraph.IsAssemblyKnownToHaveNoEvolvers was crashing on satellite
assemblies with culture identities like 'pt-br'.

Companion of Marten 8.34.1 release.
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

Successfully merging this pull request may close these issues.

3 participants