katharsis-framework icon indicating copy to clipboard operation
katharsis-framework copied to clipboard

Remove requirement for Repository classes to have a public no-arg constructor

Open timols opened this issue 8 years ago • 2 comments

In v2.8.2 of Katharsis, in order for the ReflectionsServiceDiscovery class to pick up repository classes, it requires that the class has a public no-arg constructor. I'd like to a) understand why such a thing is required if you can provide your locator service, and b) strongly urge you to remove the requirement.

The offending code is:

	private static boolean isValid(Class<?> type) {
		return !Modifier.isPrivate(type.getModifiers()) && !type.isInterface() && !Modifier.isAbstract(type.getModifiers())
				&& hasDefaultConstructor(type);
	}

	private static boolean hasDefaultConstructor(Class<?> type) {
		for (Constructor<?> contructor : type.getConstructors()) {
			if (contructor.getParameterTypes().length == 0) {
				return true;
			}
		}
		return false;
	}

I'm using Dropwizard 1.0.0 with Guice for DI, I've got everything wired together perfectly and I tend to make the constructors for my repository classes package protected so that they aren't instantiated directly. I end up with the following in my repositories:

@Singleton
public class SomeRepository extends ResourceRepositoryV2<A, B> {

    private final Service service;

    public SomeRepository() {
        throw new UnsupportedOperationException();
    }

    @Inject
    SomeRepository(Service service) {
        this.service = service;
    }
}

Failing that, perhaps throwing an exception would prevent confusion in the future?

timols avatar Feb 08 '17 03:02 timols

ServiceDiscovery would be the way to go. All the reflections stuff should really just be used if no DI environment is around. I'm not familar with Guice regarding initialization behavior. You may have a quick look at the KatharsisFeature and check what would need to change to support this. Doing the initalization lazily should not be to hard if that is necessary.

Yes I guess the above isValid function could be updated as well.

remmeier avatar Feb 09 '17 14:02 remmeier

At a minimum documenting this "feature/requirement" would be a good. I spent 4 hours while upgrading from Katharsis 2.3 to Katharsis 2.8 trying to figure out why none of my repositories would be requested.

derekbassett avatar Mar 07 '17 20:03 derekbassett