Feature request: allow more entity mapping customization

[nk-coding]

Hi,
I wanted to ask if there is any chance to mark the Neo4jMappingContext as not final.
I'm asking as I have a use case where I need to modify the creation of the PersistentEntity slightly, and this would be doable by subclassing the mapping context.
I can totally understand if you don't want to support this, however as there is really no way around this, I though I can at least give it a try asking.

[meistermeier]

What do you want to change there. Maybe we can figure out an alternative approach for your situation.
We try to keep things final because overriding functionality in those central building blocks might cause issues for users, that we want to avoid. Also, this class is pretty close to the core Spring Data mapping logic. If we have to adapt to changes there, you would not notice if you have overwritten this spot.

[nk-coding]

In this class, I would like to to overwrite createPersistentEntity and createPersistentProperty, in both cases to return a wrapper around the entity/property you return.
I mostly care about createPersistentProperty, as I would like to have the ability to implement some kind of property filter (afaik this can be done by just returning a property marked as transient if I want it to be ignored).
A little bit of background information why I want this: I have some Kotlin projects, where I use a lot of delegated properties to add some functionality (if you want to, I can go into details what these do). These delegates are stored in backing fields, which I need to ignore (I don't want them in the db). The current solution is to have @delegate:Transient on every single one of them. However, to me, this is not ideal as a) it's just annoying to always add this a bit confusing annotation, and b) if I don't pay enough attention, I use Kotlin's Transient annotation by accident (which is just a replacement for the Java transient keyword), which - in the context of Spring data - does not do anything (afaik).
So, in a nutshell, I would like to have something for ignoring properties similar to what is Neo4jConversions to conversions - you don't have to declare it for every single property, but you can do it by type.
With that being said, I know this is a very niche use case, that's the reason why I asked for allowing to modify the mapping context instead of asking directly for such a feature.

[meistermeier]

I tried several ideas based on your problem description.
Unfortunately, adding something like a SDN-only type level @Transient definition does not work. Also, modifying the classes after startup is not possible because they are already registered with all of their fields.

One option I would like to discuss is the use of projections (https://docs.spring.io/spring-data/neo4j/docs/current/reference/html/#projections):
In general, they are meant to provide a filtered/mask/limited view on the domain object themself for special use-cases.
But it might be possible in your case to see them as a kind of companion class.
If all operations are defined with this filtered type that ignores the delegate fields, it might be an option. Of course, this does only make any sense of you have not a deeply nested structure where you need to filter on every level.

[nk-coding]

First of all, thanks for trying

Unfortunately, adding something like a SDN-only type level @Transient definition does not work.

Do you mean I could not add sth like this in my project (definitely yes)? Or do you mean that it would not be possible to add sth like this to this repo? Because that would be exactly what I planned to do if the mapping context would not be final. You can find a prototype implementation of that here (relevant commit). Of course, if I would do that by overwriting, I would need to wrap the generated PersistentProperty to "overwrite" isTransient, but the idea remains the same.

Also, modifying the classes after startup is not possible because they are already registered with all of their fields.

I actually thought of this. ByteBuddy calls this "type rebasing", I could do that before the classes are passed to the MappingContext by providing a bean which does this. However, I like to not modify existing classes using bytecode manipulation (to me, this is just asking for weird behavior), and (sadly) it was a team decision to not go for that.

Regarding the use of projections:
My main issue with this approach is that I actually need the delegated properties. So even when I use a projection type (which I don't really want, that this would effectively double the classes/interfaces I need to define), I see no (good) way how I get my delegated properties back.

[michael-simons]

I don't understand @meistermeier why are you saying org.springframework.data.annotation.Transient won't work? It is exactly meant for this purpose and I just added another commit asserting it does work as intended.

As far as I understand @nk-coding it is the fact that they want to not annotate things even more but add a filter to the mapping context.

It basically is not about overriding the mapping context but adding machinery to DefaultNeo4jPersistentProperty that determines if the property is persistent transient or not.

[nk-coding]

Let me clarify a bit what I meant / want:
I don't want to add the @Transient annotation to every single field to ignore. Instead, I want to do this by the type of the field.
org.springframework.data.annotation.Transient works perfectly fine on a field, but you cannot annotate e.g. a class to ignore all fields which have this specific class as type.
I think that's what @meistermeier meant with "type level @Transient definition".
However, with that being said, I don't know why a SDN specific Transient would not work, as that's what I demonstrated here.
Regarding what I want: yes, I want some kind of customization when a property is transient. I would be perfectly fine with a type-based approach (what I demonstrated above), however there might be use cases which need a more flexible approach. However, adding some machinery to DefaultNeo4jPersistentProperty is also perfectly fine for me (as long as I have some way to actually influence what it ignores). Overriding the mapping context would just have been a way for me to add the machinery myself without you having to worry about it.

[meistermeier]

I think that's what @meistermeier meant with "type level @transient definition".

Yes, exactly. It was only about @Transient restricted to FIELD and METHOD.

[michael-simons]

I have added a PR with some ideas.

Would that work for you, @nk-coding ?

	@Bean
		public PersistentPropertyCharacteristicsProvider persistentPropertyCharacteristicsProvider() {
			return (property, owner) -> {

				if (property.getType().equals(Double.class)) {
					return PersistentPropertyCharacteristics.treatAsTransient();
				}

				return PersistentPropertyCharacteristics.useDefaults();
			};
		}

another example:

	@Test
	void characteristicsShouldBeApplied() {

		Neo4jMappingContext neo4jMappingContext1 = Neo4jMappingContext.builder().withPersistentPropertyCharacteristicsProvider((property, owner) -> {
			if (owner.getUnderlyingClass().equals(UserNode.class)) {
				if (property.getName().equals("name")) {
					return PersistentPropertyCharacteristics.treatAsTransient();
				} else if (property.getName().equals("first_name")) {
					return PersistentPropertyCharacteristics.treatAsReadOnly();
				}
			}
			if (property.getType().equals(ConvertibleType.class)) {
				return PersistentPropertyCharacteristics.treatAsTransient();
			}

			return PersistentPropertyCharacteristics.useDefaults();
		}).build();
		Neo4jMappingContext neo4jMappingContext2 = Neo4jMappingContext.builder().build();

		Neo4jPersistentEntity<?> userEntity = neo4jMappingContext1.getPersistentEntity(UserNode.class);
		assertThat(userEntity.getPersistentProperty("name")).isNull(); // Transient properties won't materialize
		assertThat(userEntity.getRequiredPersistentProperty("first_name").isTransient()).isFalse();
		assertThat(userEntity.getRequiredPersistentProperty("first_name").isReadOnly()).isTrue();

		Neo4jPersistentEntity<?> entityWithConvertible = neo4jMappingContext1.getPersistentEntity(EntityWithConvertibleProperty.class);
		assertThat(entityWithConvertible.getPersistentProperty("convertibleType")).isNull();

		entityWithConvertible = neo4jMappingContext2.getPersistentEntity(EntityWithConvertibleProperty.class);
		assertThat(entityWithConvertible.getPersistentProperty("convertibleType")).isNotNull();
	}

[nk-coding]

Yes, that would work for me perfectly.
Thanks a lot

[michael-simons]

Our pleasure, @nk-coding

I added it from 6.3.x onwards, releases are planned for January 13th (2021.2.7 and 2022.0.1).

Snapshots will be available by the end of the day I guess. If there's stuff missing, there's a bit of time slot to still add them.