satoshi
satoshi

Reputation: 4103

Autowiring a DAO into a domain object

I am coding a ribbon/achievements system for a website and I have to write some logic for each ribbon in my system. For example, you could earn a ribbon if you're in the first 2,000 people registering to the website or after 1,000 post in the forum. The idea is very similar to stackoverflow's badges, really.

So, every ribbon is obviously in the database but they also need a bit of logic to determine when a user has earned the ribbon.

In the way I coded it, Ribbon is a simple abstract class:

@Entity
@Table(name = "ribbon")
@Inheritance(strategy = InheritanceType.SINGLE_TABLE)
@DiscriminatorColumn(name = "ribbon_type")
public abstract class Ribbon
{
    @Id
    @Column(name = "id", nullable = false, length = 8)
    private int id;

    @Column(name = "title", nullable = false, length = 64)
    private String title;

    public Ribbon()
    {
    }

    public abstract boolean isEarned(User user);

    // ... getters/setters...
}

You can see I define the inheritance strategy as SINGLE_TABLE (since I have to code like 50 ribbons and I don't need additional columns for any of them).

Now, a specific ribbon will be implemented like this, for example:

@Entity
public class First2000UsersRibbon extends Ribbon
{
    @Autowired
    @Transient
    private UserHasRibbonDao userHasRibbonDao;

    public First2000UsersRibbon()
    {
        super.setId(1);
        super.setTitle("Between the first 2,000 users who registered to the website");
    }

    @Override
    public boolean isEarned(User user)
    {
        if(!userHasRibbonDao.userHasRibbon(user, this))
        {
            // TODO
            // All the logic to determine whether the user earned the ribbon
            // i.e. check whether the user is between the first 2000 users who registered to the website
            // Other autowired DAOs are needed
        }
        else
        {
            return true;
        }

        return false;
    }
}

The problem is that userHasRibbonDao is null inside the isEarned() method, so a NullPointerException is thrown.

I thought that having DAOs autowired into domain objects was wrong, but in this topic they told me that it's the correct approach (Domain-Driven Design).

I shared a non-working very simple example on GitHub: https://github.com/MintTwist/TestApp (remember to change the connection details in /WEB-INF/properties/jdbc.properties and to import the test_app.sql script)

Any help very appreciated.

Thank you!

Update - Reading the first answers, it seems like my approach is completely wrong. How would you ideally structure the code given that there may be 50-70 different ribbons? Thanks

Upvotes: 4

Views: 5225

Answers (8)

Grigorichev Denis
Grigorichev Denis

Reputation: 329

Why use DAO in the DomainObject? I suggest to decouple DAO and DomainObject, because (IMHO) the method isEarned(User user) is not relevant for First2000UsersRibbon.

class UserHasRibbonDao {
    public boolean isEarned(User user){
        if(!userHasRibbonDao.userHasRibbon(user, this)) {
        // TODO
        // All the logic to determine whether the user earned the ribbon
        // i.e. check whether the user is between the first 2000 users who registered to the website
        // Other autowired DAOs are needed
        } else {
           return true;
        }

        return false;}
}

Upvotes: 0

Sean Patrick Floyd
Sean Patrick Floyd

Reputation: 299038

One answer is missing, and it's not pretty, but it works. Instead of wiring the Dao, you can look it up from the WebApplicationContext:

RibbonDao dao = ContextLoader.getCurrentWebApplicationContext.getBean(RibbonDao.class);

This is the opposite of everything dependency injection stands for (I like to call this pattern "Inversion of Inversion of Control" :-)), but then: so is injecting services into Domain Objects.

Upvotes: 2

Chao
Chao

Reputation: 1058

I think you need to tweak your design. The first question I had was "how come your Ribbon class has the ability to check which user has it?" It's like saying the kitchen table should have a method called boolean doesThisKitchenHaveMe(Kitchen k).

It seems more logical to me that you need a 3rd locator service that maps a ribbon to an user

Upvotes: 0

Tom McIntyre
Tom McIntyre

Reputation: 3699

From what I can see the design of your hibernate classes, and the persistence of earned ribbons, is fine. I think the problem is about when and how you decide whether a User has earned a new ribbon.

Lets say a new request comes in from a logged in user. The User object is created and populated by Hibernate, and we now know all the Ribbons already earned by that User, in the userHasRibbonSet. We probably need a method like this in User:

public boolean hasEarnedRibbon(Ribbon ribbon) {
    for (UserHasRibbon userHasRibbon : userHasRibbonSet) {
        if (userHasRibbon.getRibbon().equals(ribbon) {
            return true;
        }
    }
    return false;
}

(this could probably be optimised by caching the ribbons themselves in a Set and doing a constant-time lookup, but that isn't key here)

The request is processed, the User object is updated to reflect what has happened. Then, on the way out, you check what Ribbons the User has now earned, something like this:

public class RibbonAwardingInterceptor extends HandlerInterceptorAdapter {

    @Resource
    private SessionFactory sessionFactory;
    @Resource // assuming it's a request-scoped bean; you can inject it one way or another
    private User user;

    public void postHandle(HttpServletRequest request, HttpServletResponse response, 
       Object handler, ModelAndView modelAndView) throws Exception {

        List<Ribbon> allRibbons = sessionFactory.getCurrentSession().createQuery("from Ribbon").list();

        for (Ribbon ribbon : allRibbons()  {
            if (!user.hasEarnedRibbon(ribbon)) {
                // The user has not previously earned this ribbon - lets see if they have now
                if (ribbon.isEarned(user)) {
                    user.getUserHasRibbonSet().add(new UserHasRibbon(user, ribbon));
                }
            }
        }
    }
}

If you want to use this exact pattern, make sure this interceptor goes after any interceptors that update the User in a way relevant to ribbons, but before the interceptor that closes the transaction (assuming you are using the transaction-per-request model). Then flushing the Hibernate Session will update the UserHasRibbon table automatically, so there is no real need for a dedicated DAO.

This is a simplistic approach and can obviously be refined. A clear improvement would be to be more selective with the Ribbons you are checking. Maybe each Controller method could finish by checking if any relevant Ribbons are now applicable - the Controller should know which Ribbons may be awarded after it's action.

Hope that helps, please let me know if I have completely missed the point and I'll try again.

Upvotes: 0

rodrigobartels
rodrigobartels

Reputation: 841

As Alex already mentioned is not a good practice to have your application entities as beans within your context. There are a lot of cumbersome things that can happen and it doesn't look like a good design.

The code will look something like this:

public abstract class Ribbon{

    public abstract boolean checkUser(User user);
}

public class NewUserRibbon extends Ribbon{

    @Override
    public boolean checkUser(User user){
        // your logic here
    }
}

In your Service you can have a cache collection of all the ribbons in the system (unless they are dynamic), I would suggest even to classify the ribbons by event triggers (new users, answers, votes, etc..), so you can check in your services just for the appropriate ribbons (instead of all of them), by iterating over the applicable ribbon list with the current user.

Upvotes: 0

Japan Trivedi
Japan Trivedi

Reputation: 4483

You can also try using the @Component annotation declaring along with the @Entity annotation on your First2000UsersRibbon class. And make sure that the package having this class is there in <context:component-scan base-package="" />. And along with this you need to make sure that the object of this class is not created using new operator.

Hope this helps you. Cheers.

Upvotes: 0

Alex Barnes
Alex Barnes

Reputation: 7218

I'm not saying that I agree with injecting DAOs into domain instances....but

You can wire your DAO into you domain object. But you'll have to declare your domain object in your spring application-context and obtain new instances from Spring NOT using new. Make sure that you use the prototype scope! You don't want to be getting the same singleton instance everytime!

Really this logic that you want to implement belongs in a service which is injected with the DAOs that it requires.

Perhaps you could have a service like:

@Service
public class RibbonServiceImpl implements RibbonService

  @Autowired
  private RibbonDAO ribbonDAO;

  public boolean isEarned(Ribbon ribbon, User user) {
   if(!userHasRibbonDao.userHasRibbon(user, this))
        {
            // TODO
            // All the logic to determine whether the user earned the ribbon
            // i.e. check whether the user is between the first 2000 users who registered to the website
            // Other autowired DAOs are needed
        }
        else
        {
            return true;
        }

        return false;
  }  

Upvotes: 6

Biju Kunjummen
Biju Kunjummen

Reputation: 49935

Mark it as @Configurable - @Configurable annotion will ensure that even if the beans are created outside of Spring, the dependencies are injected

You also need to add <context:spring-configured/> in your context.

Upvotes: 4

Related Questions