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.

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

Way to define fallback class when _class not found #3250

Closed
evilmilo opened this issue Feb 22, 2025 · 7 comments
Closed

Way to define fallback class when _class not found #3250

evilmilo opened this issue Feb 22, 2025 · 7 comments
Assignees
Labels
for: stackoverflow A question that's better suited to stackoverflow.com

Comments

@evilmilo
Copy link

evilmilo commented Feb 22, 2025

For a while we've had a class used in a lot of our mongo collections, FieldChange, which just recorded when a field changed. This is used in collections e.g.

class FieldChange {
   String field;
   String oldValue;
   String newValue;
}

class ExampleClass {
   List<FieldChange> changes = new ArrayList<>();
}

We can see in mongo that _class has not been written e.g. for ExampleClass in mongo:

{
    "changes" : [
        {
            "field" : "name",
            "oldValue": "Bob",
            "newValue": "Robert"
        }
    ]
}

We now are trying to enrich our model, so FieldChange becomes an abstract class, and we have many variations, but what was FieldChange is now SimpleFieldChange.

abstract class FieldChange {
   String field;
}

class SimpleFieldChange extends FieldChange {
   String field;
   String oldValue;
   String newValue;
}

But now we cannot load the existing data from mongo

org.springframework.data.mapping.model.MappingInstantiationException: Failed to instantiate example.FieldUpdateEvent using constructor public example.FieldUpdateEvent() with arguments 

We've been investigating specifying details via annotations we assumed would work, e.g.

@JsonTypeInfo(
    use = JsonTypeInfo.Id.CLASS,
    include=JsonTypeInfo.As.PROPERTY,
    property = "_class",
    defaultImpl = SimpleFieldChange.class
)
abstract class FieldChange {
   String field;
}

And a few variations on the fields as well as classes, but nothing seemed to work. We finally debugged, and can see that Mongo loads up items in the collection using DefaultMongoTypeMapper, which extends DefaultTypeMapper.

can see this has method that starts:

public <T> TypeInformation<? extends T> readType(S source, TypeInformation<T> basicType) {
	Assert.notNull(source, "Source must not be null");
	Assert.notNull(basicType, "Basic type must not be null");
	Class<?> documentsTargetType = getDefaultedTypeToBeUsed(source);
	if (documentsTargetType == null) {
		return basicType;
	}
	Class<T> rawType = basicType.getType();
        ...

effectively what would be nice is if getDefaultedTypeToBeUsed(source) had some way to then lookup by basicType if nothing in source was found for the mapping. No simple way to hook into this though, since getDefaultedTypeToBeUsed only takes source.

Would be nice if annotations on the basicType could be inspected for default implementation, this would seem to fit in with expectations.

Otherwise if their was a pluggable way to define default mappings, would seem could be put into above block, e.g.

public <T> TypeInformation<? extends T> readType(S source, TypeInformation<T> basicType) {
	Assert.notNull(source, "Source must not be null");
	Assert.notNull(basicType, "Basic type must not be null");
	Class<?> documentsTargetType = getDefaultedTypeToBeUsed(source);

	final Class<T> rawType = basicType.getType();		

	if (documentsTargetType == null) {
		// have some fallback type map that means we could lookup FieldUpdateEvent and tell it to act like it found ValueFieldUpdate
		documentsTargetType = customFallback.lookup(rawType.getName());
		if (documentsTargetType == null) {
			return basicType;
		}
	}
        ...

We're so unsure how to do this, so we're now looking to run a migration on all our data to add _class information, but feels like their should be a simpler way to configure this in code.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 22, 2025
@mp911de
Copy link
Member

mp911de commented Feb 24, 2025

TypeMapper is already the right place to handle defaulting. Have you tried subclassing DefaultMongoTypeMapper and overriding readType(S source, TypeInformation<T> basicType)?

I suggest calling readType(S source) as a first step if basicType is FieldChange and then returning SimpleFieldChange if readType(S) returns null.

@mp911de mp911de added the status: waiting-for-feedback We need additional information before we can continue label Feb 24, 2025
@evilmilo
Copy link
Author

evilmilo commented Feb 24, 2025

So I can subclass and override

	@Override
	public <T> TypeInformation<? extends T> readType(final Bson source, final TypeInformation<T> basicType) {
		final TypeInformation<? extends T> typeInformation = super.readType(source, basicType);
		final Class<? extends T> rawType = typeInformation.getType();
		final Class<?> overrideType = forceOverrideTypeMap.get(rawType);
		if (overrideType != null) {
			final Class<?> documentsTargetType = overrideType;

			boolean isMoreConcreteCustomType = (rawType == null)
				|| (rawType.isAssignableFrom(documentsTargetType) && !rawType.equals(documentsTargetType));

			if (!isMoreConcreteCustomType) {
				return basicType;
			}

			TypeInformation<?> targetType = TypeInformation.of(documentsTargetType);

			return basicType.specialize(targetType);
		}

		return typeInformation;

	}

and then I can plug that in... works. just seems very verbose and clunky.

@Configuration
public class CustomMongoConfig {

	private final MappingMongoConverter mappingMongoConverter;

	public CustomMongoConfig(final MappingMongoConverter mappingMongoConverter) {
		this.mappingMongoConverter = mappingMongoConverter;
	}

	@PostConstruct
	void initCustomMongoTypeMapper() {
		final CustomMongoConfig typeMapper = new CustomMongoConfig(
			DefaultMongoTypeMapper.DEFAULT_TYPE_KEY,
			mappingMongoConverter.getMappingContext(),
			mappingMongoConverter::getWriteTarget
		);

		mappingMongoConverter.setTypeMapper(typeMapper);
	}
}

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 24, 2025
@mp911de
Copy link
Member

mp911de commented Feb 24, 2025

Code in the comment above doesn't follow my suggestion of using readType(S source) which would allow you passing the fallback type into super.readType(source, fallbackType), but in any case it is the proper approach.

We're aiming to provide functionality for a broad audience. If certain use-cases require special handling and those aren't super-common, then we are keep our API as-is. We could introduce getFallbackTypeFor(S, TypeInformation<T>) to introduce a hook for fallback customization for future versions.

@evilmilo
Copy link
Author

oh sorry, I looked at above originaly, and it would work, but means we are looking up from source twice so seemed less efficient than checking the return type

@mp911de
Copy link
Member

mp911de commented Feb 24, 2025

The accessor uses some caching, so duplicate lookups shouldn't be expensive. There are various optimizations, e. g. you could check for _class yourself to bypass any lookup in exchange for more complexity. Let me know how it goes.

@evilmilo
Copy link
Author

Ok we got that working, you're right less complexity.

private static final Map<Class<?>, TypeInformation<?>> OVERRIDE_TYPE_MAP = new HashMap<>();

...

@Override
public <T> TypeInformation<? extends T> readType(final Bson source, final TypeInformation<T> basicType) {
    final TypeInformation<? extends T> actualTypeToUse = getActualTypeToUse(source, basicType);
    return super.readType(source, actualTypeToUse);
}

private <T> TypeInformation<? extends T> getActualTypeToUse(Bson source, TypeInformation<T> basicType) {
    if (!isClassDefinedInSource(source)) {
        final TypeInformation<?> overrideTypeToUse = OVERRIDE_TYPE_MAP.get(basicType.getType());
        if (overrideTypeToUse != null) {
            return basicType.specialize(overrideTypeToUse);
        }
    }
    return basicType;
}

private boolean isClassDefinedInSource(final Bson source) {
    final TypeInformation<?> typeFromSource = super.readType(source);
    return typeFromSource != null;
}

@mp911de
Copy link
Member

mp911de commented Feb 26, 2025

Alright, thanks for sharing. I'm closing the ticket here as there's nothing left to do for us.

@mp911de mp911de closed this as not planned Won't fix, can't repro, duplicate, stale Feb 26, 2025
@mp911de mp911de added for: stackoverflow A question that's better suited to stackoverflow.com and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Feb 26, 2025
@mp911de mp911de self-assigned this Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: stackoverflow A question that's better suited to stackoverflow.com
Projects
None yet
Development

No branches or pull requests

3 participants