Autowiring Is Good, Autowiring Is Bad
Let's say you've declared a bean using the @Service annotation:
@Service
public class MyService {
public void someLogic(PrivateInfo privateInfo) {
// super-secret business logic goes here
}
}
Then you go to autowire the bean in your @Controller to a class field:
@Controller
public class MyController {
@Autowired
MyService _myService;
}
And IntelliJ gives you the ominous, yet unhelpful, warning:
`Field injection is not recommended`
It's actually really excellent advice, but why? And what should you do instead?
Mutability vs Immutability
The Spring framework saves us a lot of trouble by automatically instantiating singleton instances of objects annotated with @Service and injecting them as dependencies into variables, functions, etc., that are annotated with @Autowired.
But the autowiring of class variables inherently requires them to be mutable (read/write), rather than immutable with the final keyword (read-only), because Spring needs to be able to write the singleton instances into the variables after the class instance has been created.
This opens the potential for a couple of different problems.
Autowired Field Initialization Failure
If the singleton @Service instance has failed to initialize properly, it may not be injected into the @Autowired field, leaving it with a NULL value that would result in errors being thrown when any code tries to access it.
Malicious or Accidental Field Injection
Worse is the possibility that the @Autowired field is modified AFTER Spring has injected the singleton @Service instance. Nothing is preventing rogue code from replacing the field with a different object that may capture and store or transmit sensitive information elsewhere, or preventing poorly-written and unreviewed code from replacing the field with a NULL or some other buggy or beta implementation that will result in failure.
class MyEvilService extends MyService {
public void someLogic(PrivateInfo privateInfo) {
this._stealPrivateInfo(privateInfo);
super.someLogic(privateInfo);
}
private void _stealPrivateInfo(PrivateInfo privateInfo) {
// I'm stealing your private info here
}
}
@Controller
public class MyController {
@Autowired
MyService _myService;
@GetMapping("/evil-request")
ResponseEntity<?> thisIsEvil() {
// Nothing stops me from doing this
this._myService = new MyEvilService();
// Now I'm stealing all your data
return ResponseEntity.ok("We're stealing now!");
}
}
Best Practice: Constructor Injection
The best practice is to inject dependencies through the class constructor that can check for any NULL values, and inject the singleton instance into an immutable final field that cannot be changed, thereby guaranteeing a much higher level of code stability and security.
@Controller
public class MyController {
final private MyService _myService;
@Autowired
MyController(MyService myService) {
if (myService == null) {
throw new RuntimeException("This should not happen");
}
this._myService = myService;
}
}
Instantiating the MyController class will fail if MyService is NULL. Attempting to overwrite _myService with something else will fail. And IntelliJ doesn't complain about it. It's a win all-around.
@Controller
public class MyController {
final private MyService _myService;
@Autowired
MyController(MyService myService) {
if (myService == null) {
throw new RuntimeException("This should not happen");
}
this._myService = myService;
}
@GetMapping("/evil-request")
ResponseEntity<?> thisIsEvil() {
// %%%%%%%%%%%%%%%%%%%%%%%%%%%%%
// I can't even compile this now
// %%%%%%%%%%%%%%%%%%%%%%%%%%%%%
this._myService = new MyEvilService();
}
}