-1

I was testing out some stuff and came across this weird behavior.

What I want to do

Let's say I want to pre-load some values into a repository using a local file, request, or whatever.

Normally I'd put these in a constructor like this:

@Service
public class PointOfInterestService {

    @Autowired
    private PointOfInterestRepository pointOfInterestRepository;

    public PointOfInterestService() {
        try {
            String jsonStr = Helper.isToString(new FileInputStream(new File("test.json")));
            JSONObject json = new JSONObject(jsonStr);
            JSONArray entries = json.getJSONArray("features");
            for (Object element : entries) {
                try {      
                        //new PointOfInterest(...) 
                        //read file and do stuff
                        //... 

                        //finally, let's save the object
                        this.registerPointOfInterest(pointOfInterest);
                    }
                } catch (Exception e) {
                    if (!(e instanceof JSONException)) {
                        e.printStackTrace();
                    }
                }

            }
        } catch (IOException e) {
            e.printStackTrace();
        }
    }

    public void registerPointOfInterest(PointOfInterest pointOfInterest) {
        pointOfInterestRepository.save(pointOfInterest);
    }
}

When the constructor runs, a NullPointerException pops up whenever registerPointOfInterest is thrown.

Using the debugger I realized that for some reason the repository is null (and thus the exception is thrown.

This is the repository class, which is pretty simple:

package com.example.demo.PointOfInterest;

import org.springframework.data.jpa.repository.JpaRepository;


public interface PointOfInterestRepository extends JpaRepository<PointOfInterest, String> {
}

Is there any easy work-around to read said file in the constructor? Thanks!

  • You can try to pass the repository in the constructor rather than have it as a field, so spring will now that have to create/inject the dependency before the @Service executes the constructor. Because the application context creates the objects randomly so it wont assure the repository object its created when the PointOfInterestService executes its constructor – Ramon jansen gomez Apr 26 '20 at 00:24
  • The autowiring can't happen until the object is already constructed. This is one of the reasons to always use constructor injection. Furthermore, this is a lot of hidden initialization to bury inside a constructor (your tests would be a nightmare!), and it would make everyone's life easier to extract the loading functionality into something like an `@Bean` method and supply the pre-parsed `List` to the constructor. – chrylis -cautiouslyoptimistic- Apr 26 '20 at 00:54
  • @chrylis-onstrike- I ended up using the `@PostConstruct` annotation. Are there any significant drawbacks? – Theodoros Tsigkanos Apr 26 '20 at 01:16
  • @TheodorosTsigkanos It still buries a lot of complex pieces in places where the logic is hard to test. There are three entirely separate steps here (loading the input stream, converting to objects, and wiring into the service) that are useful to keep distinct. Note that you should probably be using a Spring `Resource`, not `File`, and you could _probably_ do all of this in a single one-liner using Jackson with something like `mapper.readValue(resource.getInputStream(), new TypeReference>);`. – chrylis -cautiouslyoptimistic- Apr 26 '20 at 03:13
  • @chrylis-onstrike- thanks for the heads up! I'm pretty new at Spring so it's nice to get some useful feedback. Unfortunately the file I am reading is not really formatted to be parsed directly into an object (it contains a multitude of points that may or may not be nested), so it involves a bit of manual filtering. I'll definitely start reading about Resources though! – Theodoros Tsigkanos Apr 26 '20 at 04:01
  • Note that `File` is _almost always_ not the choice for a packaged application, since it doesn't read packaged resources; the Spring `Resource` API is built specifically to help automatically manage access to such things in a flexible manner. – chrylis -cautiouslyoptimistic- Apr 26 '20 at 04:19
  • @chrylis-onstrike- thanks once again! I'll definitely update the method and use appropriate project structure. – Theodoros Tsigkanos Apr 26 '20 at 04:25

3 Answers3

1

For anyone interested, I actually managed to solve this issue by deleting the constructor, and using the @PostConstruct annotation on a private method, like so:

 @PostConstruct
    private void readGeoJson() {
       ....
    }

Thanks everyone!

0

The repository will not be autowired by Spring if you have a constructor (so the service itself is not a singleton). You should change the constructor with something else.

Catalin Pirvu
  • 175
  • 2
  • 8
0

You can try this:

@Service
public class PointOfInterestService {

private PointOfInterestRepository pointOfInterestRepository;

public PointOfInterestService(
       @Autowired PointOfInterestRepository pointOfInterestRepository
) {
    this.pointOfInterestRepository =  pointOfInterestRepository;
}

Also you can remove the @Autowired in the constructors because spring knows automatically that has to do the DI.

@Service
public class PointOfInterestService {

private PointOfInterestRepository pointOfInterestRepository;

public PointOfInterestService(
       PointOfInterestRepository pointOfInterestRepository
) {
    this.pointOfInterestRepository =  pointOfInterestRepository;
}