Skip to content

Count() aggregator NoRootNodeMappingException #2633

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

Closed
goldman7911 opened this issue Nov 23, 2022 · 5 comments · Fixed by #2634
Closed

Count() aggregator NoRootNodeMappingException #2633

goldman7911 opened this issue Nov 23, 2022 · 5 comments · Fixed by #2634
Assignees
Labels
type: enhancement A general enhancement

Comments

@goldman7911
Copy link

Hi all,

I have created a replicable repo for this issue: https://github.com/goldman7911/spring-data-understanding
It will need a clear Neo4j running and run tests to validate what I'll try to explain here.

I'm starting feeling there is something wrong or my understanding is complety wrong for OGM

My data is:

{
  "identity": 171,
  "labels": [
    "RootMarker"
  ],
  "properties": {
"lastModifiedDate": 1666934940115,
"p5Latest": true,
"messageIds": [
      "fake-900b-49ac-92c7-fake",
      "fake-7777-4ab1-7777-fake"
    ],
"resources": 1,
"messageId": "fake-7777-4ab1-7777-fake",
"deviceId": "XXXXX",
"domainId": "fake-7777-4ab1-7777-fake",
"createdDate": 1666896513598,
"drniId": 222222222,
"id": 11,
"isFull": true,
"resyncId": "fake-7777-4ab1-7777-fake",
"status": "resync",
"latest": [
      22
    ]
  }
}

My objective is to map this Query to Java:

image

As you can see, all methods getResyncIdAndMessageIdsCountPureSDNXXXX are failing with same error

image

org.springframework.data.neo4j.core.mapping.NoRootNodeMappingException: Could not find mappable nodes or relationships inside Record<{resyncId: "fake-7777-4ab1-7777-fake", counter: 2}> for org.springframework.data.neo4j.core.mapping.DefaultNeo4jPersistentEntity@777d191f

Is weird how the same Neo4jRepository uses neo4jClient behind the scenes but could not map it. This method bellow I could sucessfully use in MyService.java

public Collection<MyDTO> getResyncIdandMessageIdsCount() throws NoSuchElementException {
        Collection<MyDTO> result = neo4jClient.query("match (r:RootMarker) UNWIND r.messageIds as rx return r.resyncId as resyncId, count(rx) as counter")
                .fetchAs(MyDTO.class)
                .mappedBy((typeSystem, record) -> {
                    String resyncId = record.get("resyncId").asString();
                    Long counter = record.get("counter").asLong();
                    return new MyDTO(resyncId, counter);
                }).all();
        return result;
    }

Also, just the first two of MyRepository do work. The 3 left receive the same error NoRootNodeMappingException.

public interface MyRepository extends Neo4jRepository<RootMarker, Long> {

    @Query("MATCH (n:RootMarker) RETURN n ORDER BY n.domainId")
    List<RootMarker> getRootMarker();

    @Query("MATCH (n:RootMarker) RETURN count(n)")
    Long countRootMarker();

    //THAT'S NOT WORKING
    @Query("match (r:RootMarker) UNWIND r.messageIds as rx return r.resyncId as resyncId, count(rx) as counter")
    Collection<MyDTO> getResyncIdAndMessageIdsCountPureSDN();

    //THAT'S NOT WORKING
    @Query("match (r:RootMarker) UNWIND r.messageIds as rx return r.resyncId as resyncId, count(rx) as counter")
    <T> Collection<T> getResyncIdAndMessageIdsCountPureSDNGenerics(Class<T> type);

    //THAT'S NOT WORKING
    @Query("match (r:RootMarker) UNWIND r.messageIds as rx return r.resyncId as resyncId, count(rx) as counter")
    Collection<MyProjectionDTO> getResyncIdAndMessageIdsCountPureSDNProjections();

}

Please help me understand why I can't map count aggregator result as Long by using OGM

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 23, 2022
@michael-simons
Copy link
Collaborator

Thanks for using our stuff.

It’s for the exact reason the message says: there’s a no node in your result set, just an arbitrary row. This project is not a row mapper, it’s a graph mapper.

And yes, simple solution just have a look at the fields of the record and just map them to class names works in that case but the next Ticket than will ask for nested results.

However, we will think about whether we can support this simple case with a reasonable amount of work.

@michael-simons michael-simons self-assigned this Nov 24, 2022
@goldman7911
Copy link
Author

Thanks @michael-simons for the quick reply. It is really clear by extends Neo4jRepository<THENODE, ID> that it needs to be using a actual node. I was in a kind of negation.

Seems you got the point. The next natural questions that arise is how to get nested resulted. The 'weird feeling' is that seems this project has all technology capabilities to handle that by Neo4jRepository but chosed not.

What I understand is I need create 'on-fly' a new node with a new Neo4jRepository<RESULT_NODE,ID> to fetch this result. Of course lot more work than just by neo4jClient.

I'll try to get my 'hands dirty' as a study exercise on how I think this should be and arise a PR. I think you be a fun project. In the worst I'll learn a lot. Do you have a suggestions on how should I start?

@michael-simons
Copy link
Collaborator

Hey no worries.

We spoke about that today how strict do we want to keep things in the repository thing way and let's be honest: It's not the first time a question like this has been raised.

It seems that I can make this work with little effort. You see what is necessary to shape up the content (which is also documented anyway for known entities and custom queries):

package org.springframework.data.neo4j.integration.lite;

import java.util.Collection;
import java.util.Optional;
import java.util.UUID;

import org.springframework.data.neo4j.repository.Neo4jRepository;
import org.springframework.data.neo4j.repository.query.Query;

/**
 * @author Michael J. Simons
 */
public interface SomeDomainRepository extends Neo4jRepository<SomeDomainObject, UUID> {

	/**
	 * @return Mapping arbitrary, ungrouped results into a dto
	 */
	// language=cypher
	@Query("UNWIND range(1,10) AS x RETURN randomUUID() AS resyncId, tointeger(x*rand()*10)+1 AS counter ORDER BY counter")
	Collection<MyDTO> getAllFlat();

	/**
	 * @return Mapping a single ungrouped result
	 */
	// language=cypher
	@Query("RETURN randomUUID() AS resyncId, 4711 AS counter")
	Optional<MyDTO> getOneFlat();

	/**
	 * @return Mapping a dto plus known domain objects
	 */
	// language=cypher
	@Query("""
			MATCH (u:User {login:'michael'}) -[r:OWNS] -> (s:SomeDomainObject)
			WITH u, collect(r) AS r, collect(s) AS ownedObjects
			RETURN
				u{.*, __internalNeo4jId__: id(u), r, ownedObjects} AS user,
				randomUUID() AS resyncId, 4711 AS counter,u
			""")
	Collection<MyDTO> getNestedStuff();

	/**
	 * @return Mapping nested dtos
	 */
	// language=cypher
	@Query("RETURN 'av' AS outer, {inner: 'bv'} AS nested")
	Optional<A> getOneNestedDTO();
}

michael-simons added a commit that referenced this issue Nov 24, 2022
We can support many more usecases of "I want to have a lightweight mapping tool" for DTOs by just
restricting the cases in which a `NoRootNodeMappingException` is thrown.

The idea is as follows: If the `DefaultNeo4jEntityConverter` does not deal with a synthesized
record comming from `AggregatingMappingFunction` it uses the the root map as base for mapping
if the root record is already a map value. If that is not the case and the root record does
not contain any paths that maybe aggregated later one, we synthesize a map one single time
and evaluate that for being mappable.

Closes #2633.
@michael-simons michael-simons added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 24, 2022
@michael-simons
Copy link
Collaborator

Please have a look the linked PR @goldman7911, the change is basically all in the DefaultNeo4jEntityConverter and I wrote my idea down in the commit message.

@goldman7911
Copy link
Author

Amazing @michael-simons !

michael-simons added a commit that referenced this issue Nov 24, 2022
We can support many more usecases of "I want to have a lightweight mapping tool" for DTOs by just
restricting the cases in which a `NoRootNodeMappingException` is thrown.

The idea is as follows: If the `DefaultNeo4jEntityConverter` does not deal with a synthesized
record comming from `AggregatingMappingFunction` it uses the the root map as base for mapping
if the root record is already a map value. If that is not the case and the root record does
not contain any paths that maybe aggregated later one, we synthesize a map one single time
and evaluate that for being mappable.

Closes #2633.
michael-simons added a commit that referenced this issue Nov 24, 2022
We can support many more usecases of "I want to have a lightweight mapping tool" for DTOs by just
restricting the cases in which a `NoRootNodeMappingException` is thrown.

The idea is as follows: If the `DefaultNeo4jEntityConverter` does not deal with a synthesized
record comming from `AggregatingMappingFunction` it uses the the root map as base for mapping
if the root record is already a map value. If that is not the case and the root record does
not contain any paths that maybe aggregated later one, we synthesize a map one single time
and evaluate that for being mappable.

Closes #2633.
michael-simons added a commit that referenced this issue Nov 24, 2022
We can support many more usecases of "I want to have a lightweight mapping tool" for DTOs by just
restricting the cases in which a `NoRootNodeMappingException` is thrown.

The idea is as follows: If the `DefaultNeo4jEntityConverter` does not deal with a synthesized
record comming from `AggregatingMappingFunction` it uses the the root map as base for mapping
if the root record is already a map value. If that is not the case and the root record does
not contain any paths that maybe aggregated later one, we synthesize a map one single time
and evaluate that for being mappable.

Closes #2633.
@michael-simons michael-simons mentioned this issue Jan 4, 2023
4 tasks
meistermeier pushed a commit that referenced this issue Jan 19, 2023
We can support many more usecases of "I want to have a lightweight mapping tool" for DTOs by just
restricting the cases in which a `NoRootNodeMappingException` is thrown.

The idea is as follows: If the `DefaultNeo4jEntityConverter` does not deal with a synthesized
record comming from `AggregatingMappingFunction` it uses the the root map as base for mapping
if the root record is already a map value. If that is not the case and the root record does
not contain any paths that maybe aggregated later one, we synthesize a map one single time
and evaluate that for being mappable.

Closes #2633.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants