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

Unregister ClassMap does not clear the previous maps if the same class is used #2243

Open
NicolaeSn opened this issue Mar 27, 2024 · 2 comments
Labels

Comments

@NicolaeSn
Copy link

NicolaeSn commented Mar 27, 2024

Description:
When having a class map with different mappings depending on some property when the mapping changes registering the class again with the new mapping does not take affect even though in the object the mapping is reinitialized correctly.

Code example:

public class UserMapping  :  ClassMap<UserDto> {
 public UserMapping(bool isAdmin)  {
 if(isAdmin) {
 Map(u => u.Permission);
}
else {
Map(u => u.Permissopn).Constant(Permissions.Normal);
}
 }
}

//This part here is how it is used
csv.Context.UnregisterClassMap();
var isAdmin = csv.GetField<string>(nameof(UserDto.IsAdmin));
var classMap = new UserMapping(isAdmin); //here if isAdmin is true for first user then mapping is done correctly, but if the second row is not admin the mapping is initialized correctly but internally the first Map(u => u.Permission); is still used.
csv.Context.RegisterClassMap(classMap);
var record = csv.GetRecord<UserDto>();

Expected:
Mapping is unregistered correctly.

What I've found to work as workaround:
I thought initially that splitting the UserMapping in two and then deciding which map to use would fix the problem keep in mind that both class maps where for the same UserDto class but it was not the case, somehow it seems that once map is registered for a class it is reused all over again.

What worked was to have basically a new UserDTO2 which inherits UserDTO (basically an empty class) and use that for the second class map and it worked.

Note:
Other then this problem which I think could improve the usage a lot, the library is great and worked for a pretty complex validation scenario.

@NicolaeSn NicolaeSn added the bug label Mar 27, 2024
@JoshClose
Copy link
Owner

This is because the delegate to create the objects is cached with the settings for the original map. I may need to clear those when a mapping for a type is removed, or even added a second time (updated).

For now you can have base class that does everything, and the 2 classes that don't do anything accept pass in their isAdmin true or false value.

public class BaseMapping : ClassMap<UserDto>
{
	public BaseMapping(bool isAdmin)
	{
		if (isAdmin)
		{
			Map(u => u.Permission);
		}
		else
		{
			Map(u => u.Permission).Constant(Permissions.Normal);
		}
	}
}

public class AdminMapping : BaseMapping
{
	public AdminMapping() : base(true) {}
}

public class UserMapping : BaseMapping
{
	public UserMapping() : base(false) {}
}

@NicolaeSn
Copy link
Author

I've just tested this and it s not working, I think because behind the mapping is cached for the UserDto class which is still the same even though the registered class map is different, I've tried it with inheritance and without, basically just having two different classes like:

public class AdminMapping : ClassMap<UserDto>
{
	public AdminMapping() { //some mapping here }
}

public class UserMapping : ClassMap<UserDto>
{
	public UserMapping() {// different mapping here}
}

I guess for now the only solution that works is this:

public class AdminMapping : ClassMap<UserDto>
{
	public AdminMapping() { //some mapping here }
}

public class UserMapping : ClassMap<UserDtoFake>
{
	public UserMapping() {// different mapping here}
}

public class UserDtoFake : UserDto { }

But regarding of what workaround we can use, I think the cleanest way will be to clear the cache from behind.

This is not something impactful on our side even though we use this in production we can live with the way it works now so when you have time if it's not something big to do (from the way you've described it looks like it's not) a fix for this will be a great value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants