How to alter the design so that entities don't use injections?
Asked Answered
J

4

8

I've read and came to realize myself that entities (data objects - for JPA or serialization) with injections in them is a bad idea. Here is my current design (all appropriate fields have getters and setter, and serialVersionUID which I drop for brevity).

This is the parent object which is the head of the entity composition graph. This is the object I serialize.

public class State implements Serializable {

    List<AbstractCar> cars = new ArrayList<>();

    List<AbstractPlane> planes = new ArrayList<>();

   // other objects similar to AbstractPlane as shown below
}

AbstractPlane and its subclasses are just simple classes without injections:

public abstract class AbstractPlane implements Serializable {
    long serialNumber;
}

public class PropellorPlane extends AbstractPlane {
    int propellors;
}

public class EnginePlane extends AbstractPlane {
    List<Engine> engines = new ArrayList<>(); // Engine is another pojo
}

// etc.

In contrast, each concrete type of car requires a manager that holds some behavior and also some specific form of data:

public abstract class AbstractCar implements Serializable {
    long serialNumber;

    abstract CarData getData();

    abstract void operate(int condition);

    abstract class CarData {
        String type;
        int year;
    }
}

public class Car1 extends AbstractCar {

    @Inject
    Car1Manager manager;

    Car1Data data = new Car1Data(); // (getter exists per superclass requirement)

    void operate(int i) { // logic looks weird but makes the example
        if (i < 0)
            return manager.operate(data);
        else if (i > 1)
            return manager.operate(data, i);
    }

    class Car1Data extends CarData {
        int property1;

        {
            type = "car1";
            year = 1;
        }
    }
}

public class Car2 extends AbstractCar {

    @Inject
    Car2Manager manager;

    Car2Data data = new Car2Data();

    void operate(int i) {
        if (i < 31)
            return manager.operate(data);
    }

    class Car2Data extends CarData {
        char property2;

        {
            type = "car2";
            year = 12;
        }
    }
}

// etc.

The CarxManager are @Stateless beans which perform operations on the data (the matching CarxData) given to them. They themselves further use injections of many other beans and they are all subclasses of AbstractCarManager. There are O(100) car types and matching managers.

The issue when serializing the State is that serializing the list of abstract cars does not play well with the injections in the subclasses. I'm looking for a design that decouples the injection from the data saving process.

My previous related questions: How to serialize an injected bean? and How can I tell the CDI container to "activate" a bean?

Joline answered 6/9, 2017 at 4:13 Comment(3)
The manager-network is built by CDI or EJB, yes? If each type of car has a matching manager, why not let the manager do the serialization? If there are less manager-classes than car-classes, anybody else outside the car-class has to know how to connect the managers with the cars, a kind of factory, that factory should be called during serialization and set the manager into the car. if the cars are interconnected objects, which manage the children or neighbors perhaps instead of manager a visitor should handle the creation and another visitor should perhaps handle the operate-functionalityPolygamous
@Polygamous how does it help? 1. The Cars beans still need to be serialized and they include the injection. 2. The managers aren't aware of the entity tree and their purpose is to manipulate the data. 3. I'm serializing State - how can the managers get involved? There is one manager per car and cars are not related.Joline
Sounds like you've realised you original questions were examples of the XY Problem, but to a large extent this one is too. You are still focusing on the dependency injection as the problem, rather than looking at the bigger problem you are trying to solve. Using words like "Manager" and "Data" in your class names, and using JPA to store serialised classes as byte arrays are a strong hint that your entire solution is barking up the wrong tree (in my view).Newmarket
G
2

A possibility is to remove the property, so it won't be picked up by the serializers. This can be achieved be getting it programmatically.

private Car2Manager getCar2Manager() {
  CDI.current().select(Car2Manager.class).get();
}

I would not consider this a clean solution, but it should be a workable "solution"

Also which might work is using JPA's @Transient:

@Inject
@Transient
Car2Manager manager;

I have not tested this, so it might not work.

Gretta answered 8/9, 2017 at 14:27 Comment(1)
So in the first solution I would remove the injection and inside operate I would add that line, yes? I know about the JPA @Transient but right now I'm not using JPA on that class.Joline
C
8

You can use the repository pattern. Place your business logic into a service and inject the repository (which abstracts the persistence mechanism) and manager into that. The repository hides the persistence implementation details from the business service and the entities are just simple POJOs.

It would look something like the below with Foo being the id of the entity Bar:

public class CarService {

    @Inject
    CarRepository carRepository;

    @Inject
    CarManager manager;

    piblic void operate(final Foo foo) {

        Bar myBar = carRepository.retrieve(foo);
        manager.doSomethingTo(myBar);
        carRepository.persist(myBar);

    }
}

See also: Repository Pattern Step by Step Explanation, http://deviq.com/repository-pattern/. Some frameworks such as Spring Data JPA or deltaspike already implement the repository pattern for you, all you need to do is provide an interface like the following and they generate the implementation in the background:

@Repository
public interface CarRepository extends EntityRepository<Car, UUID> {}

Mark in answer to your request for more detail I am going to provide a remodeled solution because the example in the question really did not make sense to me and exhibits quite a few anti-patterns which lead to problematic software.

To find a good solution to the problem touches on a lot of different considerations, many of which are very large topics with many books written about them, but I will try my best to illustrate my thinking based on these to solve the above problem.

And apologies as I have no doubt you are aware of many of these, but I shall assume limited knowledge for the sake of clarity.

The first step in solving this problem is not about code, but about the model itself, model driven development is covered extensively in Eric Evan's book as mentioned in the comments below. The model should drive the implementation and should also exist on its own tier as part of a layered architecture and is made up of entities, value objects and factories.

Model Driven Development

In the model given in the question we have something called a State, which contains AbstractPlanes and AbstractCars. You are using JPA to persists the State which is effectively an aggregate of your planes and cars. Firstly calling anything a State in software is a bad smell because pretty much everything has some sort of state, but calling what we have here which is an aggregate the State makes even less sense.

How does one State differ from another? Is one car part of one State and another part of a different State or is it the case that all planes and cars belong to a single instance of State. What is the relationship between planes and cars in this scenario? How does a list of planes and a list of cars have any relation to a single State entity?

Well if State was actually an Airport and we were interested in how many planes and cars were currently on the ground, then this could be the correct model. If State was an Airport it would have a name or identity such as its airport code, but it does not and so...

... in this case, it seems that State is an object which is being used as a convenience to allow us to access the object model. So we are effectively driving our model by implementation considerations, when we should doing it the other way round and driving our implementation from our model.

Terms like CarData are also problematic for the same reason, creating a Car entity and then a separate object to store its Data is messy and confusing.

Failure to get the model right results in software that is at best confused and at worst completely non-functional. This is one of the largest causes of failed IT programmes and the bigger the project the harder this stuff is to get right.


Revised Model

So from the model I understand that we have Cars and we have Planes, instances of which are all unique entities with their own identity. They seem to me to be separate things and so there is no point in persisting them wrapped in some aggregate entity.

public class Plane {...}

public class Car {...}

Another consideration is the use of abstract classes in the model, generally we want to apply the principle of favoring composition over inheritance because inheritance can result in hidden behaviors and it can make a model hard to read. For example why have we got a ProperllerPlane and an EnginePlane? Surely a propeller is just a type of engine? I have greatly simplified the model:

public class Plane implements Serializable {

    @Id
    private String name;
    private String model;
    private List<Engine> engines;

The Plane is an entity with its own attributes and identity. There is no need to have additional classes which represent nothing in the real world just to store attributes. The engine object is currently an enum representing the type of engine used in the plane:

public enum Engine {
    PROPELLER, JET
}

If the engine itself were to require an identity, as in real life engine serial numbers and things are tracked, then we would change this to an object. But we might not want to allow access to it except through a Plane entity instance, in which case the Plane will be known as a aggregate root - this is an advanced topic and I would recommend Evan's book for more details on aggregates.

The same goes for the Car entity.

@Entity
public class Car implements Serializable{

    @Id
    private String registration;
    private String type;
    private int year;

The above is all you need from what was provided in the question for the basis of your model. I have then created a couple of factory classes which handle creation of instances of these entities:

public class CarFactory {

    public Car makePosrche(final String registrationNumber) {
        Car porsche = new Car();
        porsche.setRegistration(registrationNumber);
        porsche.setType("Posrshe");
        porsche.setYear(1986);
        return porsche;
    }
}

public class PlaneFactory {

    public Plane makeSevenFourSeven(final String name) {
        Plane sevenFourSeven = new Plane();
        List<Engine> engines = new ArrayList<Engine>();
        engines.add(JET);
        engines.add(JET);
        engines.add(JET);
        engines.add(JET);
        sevenFourSeven.setEngines(engines);
        sevenFourSeven.setName(name);
        return sevenFourSeven;
    }

    public Plane makeSpitFire(final String name) {
        Plane spitFire = new Plane();
        List<Engine> engines = new ArrayList<Engine>();
        engines.add(PROPELLER);
        spitFire.setEngines(engines);
        spitFire.setModel("Spitfire");
        spitFire.setName(name);
        return spitFire;
    }
}

What we are also doing here is separating out concerns as according to the Single Responsibility Principle each class should only really do one thing.


Now that we have a model we need to know how to interact with it. In this case we would most likely if using JPA persist the Cars in a table called Car and the Planes likewise. We would provide access to these persisted entities via repositories, CarRepository and PlaneRespository.

You can then create classes called services which inject the repositories (and anything else you require) to perform CRUD (Create Read Update Delete) operations on the instances of cars and planes and also this is the point where you can apply your business logic to these. Such as your method:

void operate(int i) {..}

By structuring your code this way you decouple the model (entities and value objects) from how they are persisted (repositories) from the services which operate on them as mentioned in your question:

I'm looking for a design that decouples the injection from the data saving process.

Custodial answered 13/9, 2017 at 12:50 Comment(7)
I agree that this is the best approach. For a very detailed overview of this design pattern, check out the great Eric Evans book, Domain Driven Design (amazon.com/Domain-Driven-Design-Tackling-Complexity-Software/dp/…).Moslemism
I don't understand this answer. Can you show how it would work with my classes?Joline
Hi Mark, if you replace Bar for Car then you basically have it, if I get time this week I'll try and add a bit more detail.Custodial
Hi Justin, Iv'e given you the bounty based on popular opinion but still can't completely see how you would recreate my intended behavior. For example, why am I persisting a single car in the operate method which is just supposed to change its data? Persistence is called on the whole State class whenever it is needed. Also what is State holding a list of? CarRepositorys? If so, then how do I create CarServices? I hope you could take the time to rearrange my given code in your way. Thanks.Joline
Thanks for the detailed answer but there are a few discrepancies. My CarData objects have different fields. In addition to the data in AbstractCar, Car1Data has an int field and Car2Data has a char field. The respective managers need those fields for the business logic. This is why I have different classes for each car and coupled to it its specialized manager. Also cars and planes are not JPA entities, I serialize them and then use JPA to store the byte array but I don't think it matters here.Joline
And about State. Yes, it's like an Airport. It just holds the data of all the (unrelated) cars and plains. This object is being sent from the server to the client. If I don't aggregate all the data of the Airport I need to send each one separately which is a big problem. There are many Airport objects and different clients request different instances.Joline
Thanks Mark, I think the above answer is all I can add on this topic. Good luck with your project and if you can consider some of the principles in the answer it will definitely be of help.Custodial
G
2

A possibility is to remove the property, so it won't be picked up by the serializers. This can be achieved be getting it programmatically.

private Car2Manager getCar2Manager() {
  CDI.current().select(Car2Manager.class).get();
}

I would not consider this a clean solution, but it should be a workable "solution"

Also which might work is using JPA's @Transient:

@Inject
@Transient
Car2Manager manager;

I have not tested this, so it might not work.

Gretta answered 8/9, 2017 at 14:27 Comment(1)
So in the first solution I would remove the injection and inside operate I would add that line, yes? I know about the JPA @Transient but right now I'm not using JPA on that class.Joline
S
0

What is the entry point? Is this a web application, a rest service, a soap service, or event a scheduler?

Injection frameworks almost always separate data and service. Data are always POJO, containing absolutely no business logic. Here, assuming this is a rest-service, i will do the following:

public class SSOApplication {

    public class State implements Serializable {

        List<AbstractCar> cars = new ArrayList<>();

        List<AbstractPlane> planes = new ArrayList<>();

        // other objects similar to AbstractPlane as shown below
    }

    public abstract class AbstractPlane implements Serializable {

        long serialNumber;
    }

    public class PropellorPlane extends AbstractPlane {

        int propellors;
    }

    public class EnginePlane extends AbstractPlane {

        List<Engine> engines = new ArrayList<>(); // Engine is another pojo
    }

    public abstract class AbstractCar implements Serializable {

        long serialNumber;

        abstract CarData getData();

    }

    public static class CarData {

        String type;
        int year;
    }

    public class Car2Data extends CarData {

        char property2;

        {
            type = "car2";
            year = 12;
        }
    }

    public static class Car1Data extends CarData {

        int property1;

        {
            type = "car1";
            year = 1;
        }
    }

    public static class Car1 extends AbstractCar {

        @Override
        CarData getData() {
            throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates.
        }

    }

    public static class Car2 extends AbstractCar {

        @Override
        CarData getData() {
            throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates.
        }

    }

    public static interface CarManager<T extends CarData> {

        void operate(T car, int index);

        default boolean canHandle(T carData) {
            final TypeToken<T> token = new TypeToken<T>(getClass()) {
            };

            return token.getType() == carData.getClass();
        }
    }

    @ApplicationScoped
    public static class Car1Manager implements CarManager<Car1Data> {

        public void operate(Car1Data car, int index) {
        }
    }

    @ApplicationScoped
    public static class Car2Manager implements CarManager<Car2Data> {

        public void operate(Car2Data car, int index) {
        }
    }

    @ApplicationScoped
    public static class CarService {

        @Any
        @Inject
        private Instance<CarManager<?>> carManagers;

        public void operate(int index, AbstractCar car) {
            final CarData carData = car.getData();
            final CarManager<?> carManager = carManagers.stream()
                    .filter((mng) -> mng.canHandle(carData))
                    .findFirst()
                    .orElse(IllegalArgumentException::new);

            carManager.operate(carData, index);
        }
    }
}
Superstition answered 8/9, 2017 at 16:4 Comment(4)
It's JAX-RS. Why are all the classes inner classes of SSOApplication?Joline
I was just putting them together so that I can copy pasteSuperstition
Why @ApplicationScoped? Won't it be worse than @Stateless? And streaming over all managers to find a match each time an operation needs to be handled seems inefficient.Joline
All that data is in memory. Unless you are targeting to have a million car managers. As to @ApplicationScoped, i was just activating annotated bean. You can use @Stateless provided it is a no-interface view, otherwise you will have to add both @Stateless and @Dependent to the stateless bean.Superstition
C
0

If you could alter your flow than perhaps you could do something like this:

class Car1InnerService {

    @Inject
    Car1Manager manager;

    void operate(int i, Car1 car) { 
     if (i < 0)
        return manager.operate(car.getData());
     else if (i > 1)
        return manager.operate(car.getData(), i);
     }
   }
}

I introduced some inner service which will operate on Car1 and use Car1Manager for it. Your AbstractCar class will also of course lose it's operate method because from now on your service will handle it. So now instead of calling car1.operate(i) you will have to make a call via Service like this:

public class SampleCar1ServiceUsage{
    @Inject
    Car1InnerService car1InnerService;

    public void carManipulator(List<Car1> carlist){
        int i = 0; //I don't know why you need this param therefore i just increment it 
        for(Car1 car: carlist){
           car1InnerService.operate(i, car);
           i++;
        }
    }   
}

Of course you should introduce similar functionality for every other AbsractCar children (perhaps even extract some abstraction if necessary like for example AbsractCarInnerService which would define operate method or some interface which would do the same if you don't want any other solid methods in it). However this answer is still somehow related to @Justin Cooke answer and in my opinion you should definitely check those patterns which he mentioned in his post.

Corr answered 14/9, 2017 at 14:19 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.