neo4j / neo4j-ogm

Java Object-Graph Mapping Library for Neo4j
https://neo4j.com/docs/ogm-manual/
Apache License 2.0
337 stars 165 forks source link

Disabling cache (feature request) #478

Closed elenigen closed 5 years ago

elenigen commented 6 years ago

It would be nice to have a simple configuration to put in place to disable any caching, which would be very useful for testing. In our test, we discovered that we need to keep calling session.clear() after the setup of a test, or the response will contain cached objects which were not part of the Cypher query RETURN clause.

Expected Behavior

If I persist a node with it's "neighbors" and then I execute a Cypher query where only one node is targeted, then I should not receive other nodes than the one requested.

session = sessionFactory.openSessionWithoutCaching()
nodeA.nodeB = nodeB
session.save(nodeA)
queriedA = session.queryForObject(NodeA.class, " MATCH ... RETURN nodeA", map)
assertThat(queriedA.nodeB).isNull()

Current Behavior

session = sessionFactory.openSession()
nodeA.nodeB = nodeB
session.save(nodeA)
session.clear()  <= EXTRA CLEAR NEEDED
queriedA = session.queryForObject(NodeA.class, " MATCH ... RETURN nodeA", map)
assertThat(queriedA.nodeB).isNull()
elenigen commented 6 years ago

My current workaround (in Kotlin):

class AutoClearSession(val defaultNeo4jSession: Session) : Session by defaultNeo4jSession {

    override fun <T, ID : Serializable> loadAll(type: Class<T>, ids: Collection<ID>): Collection<T> {
        return defaultNeo4jSession.loadAll(type, ids)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T, ID : Serializable> loadAll(type: Class<T>, ids: Collection<ID>, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(type, ids, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T, ID : Serializable> loadAll(type: Class<T>, ids: Collection<ID>, sortOrder: SortOrder): Collection<T> {
        return defaultNeo4jSession.loadAll(type, ids, sortOrder)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T, ID : Serializable> loadAll(type: Class<T>, ids: Collection<ID>, sortOrder: SortOrder, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(type, ids, sortOrder, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T, ID : Serializable> loadAll(type: Class<T>, ids: Collection<ID>, pagination: Pagination): Collection<T> {
        return defaultNeo4jSession.loadAll(type, ids, pagination)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T, ID : Serializable> loadAll(type: Class<T>, ids: Collection<ID>, pagination: Pagination, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(type, ids, pagination, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T, ID : Serializable> loadAll(type: Class<T>, ids: Collection<ID>, sortOrder: SortOrder, pagination: Pagination): Collection<T> {
        return defaultNeo4jSession.loadAll(type, ids, sortOrder, pagination)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T, ID : Serializable> loadAll(type: Class<T>, ids: Collection<ID>, sortOrder: SortOrder, pagination: Pagination, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(type, ids, sortOrder, pagination, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(objects: Collection<T>): Collection<T> {
        return defaultNeo4jSession.loadAll(objects)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(objects: Collection<T>, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(objects, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(objects: Collection<T>, sortOrder: SortOrder): Collection<T> {
        return defaultNeo4jSession.loadAll(objects, sortOrder)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(objects: Collection<T>, sortOrder: SortOrder, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(objects, sortOrder, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(objects: Collection<T>, pagination: Pagination): Collection<T> {
        return defaultNeo4jSession.loadAll(objects, pagination)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(objects: Collection<T>, pagination: Pagination, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(objects, pagination, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(objects: Collection<T>, sortOrder: SortOrder, pagination: Pagination): Collection<T> {
        return defaultNeo4jSession.loadAll(objects, sortOrder, pagination)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(objects: Collection<T>, sortOrder: SortOrder, pagination: Pagination, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(objects, sortOrder, pagination, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>): Collection<T> {
        return defaultNeo4jSession.loadAll(type)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(type, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, sortOrder: SortOrder): Collection<T> {
        return defaultNeo4jSession.loadAll(type, sortOrder)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, sortOrder: SortOrder, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(type, sortOrder, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, pagination: Pagination): Collection<T> {
        return defaultNeo4jSession.loadAll(type, pagination)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, pagination: Pagination, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(type, pagination, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, sortOrder: SortOrder, pagination: Pagination): Collection<T> {
        return defaultNeo4jSession.loadAll(type, sortOrder, pagination)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, sortOrder: SortOrder, pagination: Pagination, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(type, sortOrder, pagination, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, filter: Filter): Collection<T> {
        return defaultNeo4jSession.loadAll(type, filter)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, filter: Filter, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(type, filter, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, filter: Filter, sortOrder: SortOrder): Collection<T> {
        return defaultNeo4jSession.loadAll(type, filter, sortOrder)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, filter: Filter, sortOrder: SortOrder, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(type, filter, sortOrder, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, filter: Filter, pagination: Pagination): Collection<T> {
        return defaultNeo4jSession.loadAll(type, filter, pagination)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, filter: Filter, pagination: Pagination, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(type, filter, pagination, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, filter: Filter, sortOrder: SortOrder, pagination: Pagination): Collection<T> {
        return defaultNeo4jSession.loadAll(type, filter, sortOrder, pagination)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, filters: Filters): Collection<T> {
        return defaultNeo4jSession.loadAll(type, filters)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, filters: Filters, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(type, filters, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, filters: Filters, sortOrder: SortOrder): Collection<T> {
        return defaultNeo4jSession.loadAll(type, filters, sortOrder)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, filters: Filters, sortOrder: SortOrder, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(type, filters, sortOrder, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, filters: Filters, pagination: Pagination): Collection<T> {
        return defaultNeo4jSession.loadAll(type, filters, pagination)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, filters: Filters, pagination: Pagination, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(type, filters, pagination, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, filters: Filters, sortOrder: SortOrder, pagination: Pagination): Collection<T> {
        return defaultNeo4jSession.loadAll(type, filters, sortOrder, pagination)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> loadAll(type: Class<T>, filters: Filters, sortOrder: SortOrder, pagination: Pagination, depth: Int): Collection<T> {
        return defaultNeo4jSession.loadAll(type, filters, sortOrder, pagination, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T, ID : Serializable> load(type: Class<T>, id: ID): T {
        return defaultNeo4jSession.load(type, id)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T, ID : Serializable> load(type: Class<T>, id: ID, depth: Int): T {
        return defaultNeo4jSession.load(type, id, depth)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> save(objectToSave: T) {
        defaultNeo4jSession.save(objectToSave)
        defaultNeo4jSession.clear()
    }

    override fun <T> save(objectToSave: T, depth: Int) {
        defaultNeo4jSession.save(objectToSave, depth)
        defaultNeo4jSession.clear()
    }

    override fun <T> queryForObject(objectType: Class<T>, cypher: String, parameters: Map<String, *>): T {
        return defaultNeo4jSession.queryForObject(objectType, cypher, parameters)
            .also { defaultNeo4jSession.clear() }
    }

    override fun <T> query(objectType: Class<T>, cypher: String, parameters: Map<String, *>): Iterable<T> {
        return defaultNeo4jSession.query(objectType, cypher, parameters)
            .also { defaultNeo4jSession.clear() }
    }

    override fun query(cypher: String, parameters: Map<String, *>): Result {
        return defaultNeo4jSession.query(cypher, parameters)
            .also { defaultNeo4jSession.clear() }
    }

    override fun query(cypher: String, parameters: Map<String, *>, readOnly: Boolean): Result {
        return defaultNeo4jSession.query(cypher, parameters, readOnly)
            .also { defaultNeo4jSession.clear() }
    }
}
meistermeier commented 6 years ago

Why is this such a problem for testing? If you want to keep the current Session it might be easier to just clear it in the setup (e.g. @Before method in JUnit). The idea behind the caching is that it is much cheaper regarding performance when the same entity get returned by the query. This is one of the reasons I do not see any benefit for production code if we would provide an additional API.

elenigen commented 6 years ago

The @Before solution is only applicable in few situations, because usually in a unit test within the @Test function, you take care of also preparing the data for your current test, while the @Before is to isolate tests. The current issue with the caching is the fact we are pushing data first in the DB, then we trigger the test and since we just pushed the data, then it's available in the cache... which in real life in production, the data might be already there, so we won't hit the cache. I can tell you, we discovered many bugs by disabling the cache, so I truly it's a useful feature in test.

The first solution that I had in mind we pretty simple, I would simply flush the call every time I finished to prepare the data for the current test, but that means all dev needs to be aware of this workaround and they all need to remember it in every single tests, which is far from safe practice. That's why I had to wrap the Session.

meistermeier commented 6 years ago

Ok, I got your point. Might be influenced by my style of writing tests: avoid data initialisation within the @Test methods and do write them once for the whole test class in the @Before method. Let me see what I can do here. I would like to avoid to bring this as part of the 'production' code but also not include it in neo4j-ogm-test because there will be people who just want to have this one functionality.

elenigen commented 6 years ago

I don't know if you could provide a UncachedSession class, so we can choose and in prod, nobody would use this one.

meistermeier commented 6 years ago

I am still struggling with myself here. An UncachedSession will solve the test design problem for now but introduce wrong behaviour regarding data manipulation.

For example

MyEntity myEntity = uncachedSession.load(.....); // some entity with 1:1 relationship
myEntity.setRelationship(...);
uncachedSession.save(myEntity);

the session will not "see" the change that happens here because it is completely unaware of the previously existing relationship and happily create a new one. This will result in two relationships with a 1:1 defined relation in the domain model but 1:n in the database.

meistermeier commented 5 years ago

I close this issue now because there were no further responses. Mostly because of my previous comment I see no clean solution to provide for tests.

elenigen commented 5 years ago

Since it's only for test purposes, I would guess it's not a big deal to do an extra lookup before creating new nodes. In the end, we are no longer using Neo4j, so it's not something I would really need. Thanks any way!