5

Warning: I realize this may be an abuse of the intention behind spring beans and/or Java 8's default interface methods. What I'm looking for is specific and reasoned criticisms of why this might be an unsafe approach that I am failing to recognize.

I've defined a class that gives me static access to the running application context:

@Component
public class BeanAccessor implements ApplicationContextAware {

    private static ApplicationContext applicationContext;

    public static <T> T getSingleton(Class<T> clazz){
        return applicationContext.getBean(clazz);
    }

    public static <T> T getSingleton(String beanName, Class<T> clazz){
        return applicationContext.getBean(beanName, clazz);
    }

    @Override
    public void setApplicationContext(ApplicationContext applicationContext) throws BeansException {
        BeanAccessor.applicationContext = applicationContext;
    }

}

I then am able to use those BeanAccessor methods inside of default interface methods in order to gain access to spring managed beans from inside the method.

I realize that this functionality could be achieved by implementing other classes and services rather than feeling a need to mix it into the current one (at least for the example below, I'm not actually doing anything with 'this'). This is partly an 'artistic' preference and I'm sure I'll continue to find other use cases.

Note that I am NOT trying to preserve state per instance like described here https://kerflyn.wordpress.com/2012/07/09/java-8-now-you-have-mixins/ and I do recognize that there are dangers in doing that.

Here's an example usage:

public interface ClientAware {

    String TENANT_NAME = "TENANT_NAME";

    default ClientDetails clientDetails() {
        ClientDetailsService service = BeanAccessor.getSingleton(ClientDetailsService.class);
        return service.loadClientByClientId(SecurityUtil.getClientId());
    }

    default Map<String, Object> clientInfo() {
        return clientDetails().getAdditionalInformation();
    }

    default String tenant() {
        return (String) clientInfo().get(TENANT_NAME);
    }

}

And then my actual class using the interface (and another interface that does similar things to add meta-information to the response):

@RestController
@RequestMapping("/documents")
public class Documents implements WrapAware, ClientAware {

        @Autowired
        private DocumentService docService;

        @RequestMapping(method = RequestMethod.GET)
        public Object byPathAndTenant(@RequestParam("path") String path) {
            return ok(docService.getDocumentsByPathAndTenant(path, tenant()));
        }

}

Note that it does resolve the beans appropriately and seems to work. Before I start utilizing a pattern like this I would like to be aware of the risks.

RutledgePaulV
  • 2,568
  • 3
  • 24
  • 47
  • That’s an abuse of an `interface` as a namespace. Since these `default` methods do nothing but return static information, you should make them `static` and use `import static` instead of implementing a pseudo-`interface`. – Holger Jun 11 '15 at 13:34
  • @Holger, I realize for this example that is true. But say that I wanted to add functionality that needs access to the current class instance? In that way I gain significant functionality simply by adding on an interface and avoid having to pass the instance to other methods. – RutledgePaulV Jun 11 '15 at 18:44
  • Well, in this example, it’s also unclear why you can’t simply use an abstract base class for `Documents`. Besides that, there is nothing wrong with using inheritance, regardless of whether via `abstract class` or `interface` with `default` methods. Or is there a concrete concern why this should be different in the context of Spring? – Holger Jun 11 '15 at 18:49
  • No, not really. I think some of application context stuff still feels like magic (if you know any great articles, I'd appreciate it). It also just feels like a bit of a hack in order to get 'trait' like functionality so I wanted to be careful about it before I go around using it just because I like it :) – RutledgePaulV Jun 11 '15 at 18:51
  • 1
    Maybe [this Q&A](http://stackoverflow.com/q/28681737/2711488) helps… – Holger Jun 11 '15 at 18:56

1 Answers1

1

There are the already discussed caveats of on staying away from statics

In your case, you may face an issue if the interface gets referenced before your spring context is initialized. So the static ClientDetailsService service would be null resulting in an NPE.

Generally as the code grows, you may lose control of when this interface is referenced during app startup.

Community
  • 1
  • 1
6ton
  • 4,174
  • 1
  • 22
  • 37
  • 1
    Good point [the thing with the initialization order](http://stackoverflow.com/q/23096084/2711488). But it’s also wrong to put static information into `interface` methods, making it look like per-instance information… – Holger Jun 11 '15 at 13:43
  • @Holger in my mind when I think of default interfaces, I am still stuck on statics rather than per instance. Though I get your ponit on how the semantics involved may lead someone to believe its instance level info vs static. – 6ton Jun 11 '15 at 13:48
  • 2
    Please, `default` *methods*. The presence of `default` methods does not change what `interface`s are, so there are no “default interfaces”. If you want `static` methods, you may add `static` methods to an `interface` (which doesn’t make it a “static interface”). But `default` methods are just non⁠-⁠`abstract` methods just like other non-`abstract` instance methods on concrete `class`es. – Holger Jun 11 '15 at 13:56
  • @Holger yup agree, that comment is incorrect. Thanks for correcting – 6ton Jun 11 '15 at 15:27
  • So, I've modified the code a bit to what I originally had (I had moved the services out to be fields when I first posted the question). I think I will avoid the initialization order problem by just always getting the bean inside of the default method. While I realize this trivial example can be replaced with static fields on any class, the thing I gain here is access to 'this'. Think: models that have a .persist() method that actually persists them to a database, etc. – RutledgePaulV Jun 11 '15 at 18:41