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

Use of other custom ObjectMappers broken by gross reflection hack #1

Closed
dualspiral opened this issue Jun 3, 2020 · 1 comment
Closed

Comments

@dualspiral
Copy link

dualspiral commented Jun 3, 2020

static {
try {
Field field = TypeSerializers.getDefaultSerializers()
.getClass()
.getDeclaredField("serializers");
field.setAccessible(true);
CopyOnWriteArrayList serializers =(CopyOnWriteArrayList) field.get(TypeSerializers.getDefaultSerializers());
serializers.remove(3);
} catch (IllegalAccessException e) {
e.printStackTrace();
} catch (NoSuchFieldException e) {
e.printStackTrace();
}
TypeSerializers.getDefaultSerializers()
.registerPredicate(intput -> intput.getRawType().isAnnotationPresent(ConfigSerializable.class),
new ClassTypeNodeSerializer());
}

This is completely unnecssary and prevents other users from using their own object mapper without having to resort to their own hacks/workarounds. For example, you are breaking Nucleus. You should not be forcing people to use your object mapper like you do here.

Rather than ripping out a specific serialiser, you should:

  • Remove the entire static block linked to above
  • Create a child TypeSerializerCollection - TypeSerializers.getDefaultSerializers().newChild()
  • Add your custom serialiser to that (child serialisers have higher priority than their parents)
  • Use this child serialiser collection when creating a loader (via the ConfigurationOptions#withSerializers in 3.7, setSerializers prior to that). While you're there, you should create an ObjectMapperFactory for your object mapper and set it in those options too.

That way, the projects that want to use your extensions can, while those that don't aren't forced to anyway. I get that you might see this as completely compatible with the standard object mapper - that's fine, but it's not compatible with my object mapper. Please stop forcing people to use your mapper when this project is being used.

@NeumimTo
Copy link
Member

NeumimTo commented Jun 5, 2020

@dualspiral thanks for reporting, im aware of this but i no longer need this library in my projects so i cannot say when or if this gets ever fixed.
Tbh best option would be to put these features to configurate

It was a fast hack to make it work as i wanted.

Eventually i discovered that former version of configurate lacks too much features i was hoping for, so i switched to nightconfig.

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

No branches or pull requests

2 participants