The first question you need to answer is whether you intend to create a truly generic data access framework, or if you want it tailored to the particular project you're working on.
A generic framework has the benefit of being portable and fairly loosely coupled to the underlying DB + schema, but at the cost of pushing schema level information further up in the framework. The more generic you make the framework, the more of this knowledge you push up.
A tailored framework alleviates the higher levels of the application from worrying about the schema or DB details, but at the cost of cost of being less re-usable in other environments. As a corollary, the more specific you make the framework, the less easy it is to extend elsewhere.
If you're not sure on which of those to pick, the generic framework makes sense if you suspect there will be changes at the DB layer or the schema. Likewise, if those are pretty well locked down then the tailored framework is the better approach. Ultimately, you're optimizing the system to more easily handle where you think the change will occur.
As to whether or not you should refactor - the answer is No unless you're encountering a specific problem.
Worrying about the degree of coupling is nice, I suppose, but doesn't necessarily address a specific problem that you may be seeing. Refactoring based upon presumed issues within coupling is actually more likely to create a problem for you than just leaving it alone.
Likewise with expressing the state. If the current mechanism of an enum
is sufficient and doesn't present an apparent problem then don't change what you've got.
Overall, it sounds like you've designed a reasonably robust data access method for the domain you're working worth. Your question doesn't list any specific issues that are holding back your development, so I'd call it good and move to the next stage of development.
Your case
In your case all three options are viable. I think that the best option is probably to sync your data sources someplace the asp.net application is not even aware of. That is, avoid the two fetches in the foreground every time, sync the API with the db silently). So if that's a viable option in your case - I say do it.
A solution where you make the fetch 'once' like the other answer suggests doesn't seem very viable since it doesn't persist the response anywhere and ASP.NET MVC will just make the fetch for every request over and over.
I'd avoid the singleton, I don't think it's a good idea at all for plenty of the usual reasons.
If the third option is not viable - one option is to lazy load it . That is, have a class extend the entity, and have it hit the API on a need to basis. That's a very dangerous abstraction though since it's even more magic and non-obvious state.
I guess it really boils down to several questions:
- How often does the API call data change? Not often? Third option. Often? Suddenly the third option is not too viable. I'm not sure I'm as against ad-hoc calls as you.
- How expensive is an API call? Do you pay per call? Are they fast? Free? If they're fast, making a call each time might work, if they're slow you need to have some sort of prediction in place and make the calls. If they cost money - that's a big incentive for caching.
- How fast does the response time have to be? Obviously faster is better, but sacrificing speed for simplicity might be worth it in some cases, especially if it's not directly facing a user.
- How different is the API data from your data? Are they two conceptually different things? If so, it might be even better to just expose the API outside rather than return the API result with the result directly and let the other side make the second call and handle managing it.
A word or two about separation of concerns
Allow me to argue against what Bobson is saying about separation of concerns here. At the end of the day - putting that logic in the entities like that violates separation of concerns just as bad.
Having such a repository violates separation of concerns just as bad by putting presentation centric logic in the business logic layer. Your repository is now suddenly aware of the presentation related things like how you display the user in your asp.net mvc controllers.
In this related question I've asked about accessing entities directly from a controller. Allow me to quote one of the answers there:
"Welcome to BigPizza, the custom Pizza shop, may I take your order?" "Well, I'd like to have a Pizza with olives, but tomato sauce on top and cheese at the bottom and bake it in the oven for 90 minutes until it's black and hard like a flat rock of granite." "OK, Sir, custom Pizzas are our profession, we'll make it."
The cashier goes to the kitchen. "There is a psycho at the counter, he wants to have a Pizza with... it's a rock of granite with ... wait ... we need to have a name first", he tells the cook.
"No!", the cook screams, "not again! You know we tried that already." He takes a stack of paper with 400 pages, "here we have rock of granite from 2005, but... it didn't have olives, but paprica instead... or here is top tomato ... but the customer wanted it baked only half a minute." "Maybe we should call it TopTomatoGraniteRockSpecial?" "But it doesn't take the cheese at the bottom into account..." The cashier: "That's what Special is supposed to express." "But having the Pizza rock formed like a pyramid would be special as well", the cook replies. "Hmmm ... it is difficult...", the desparate cashier says.
"IS MY PIZZA ALREADY IN THE OVEN?", suddenly it shouts through the kitchen door. "Let's stop this discussion, just tell me how to make this Pizza, we are not going to have such a Pizza a second time", the cook decides. "OK, it's a Pizza with olives, but tomato sauce on top and cheese at the bottom and bake it in the oven for 90 minutes until it's black and hard like a flat rock of granite."
(Read the rest of the answer, it's really nice imo).
It's naive to ignore the fact there is a database - there is a database, and no matter how hard you want to abstract that, it's not going anywhere. Your application will be aware of the data source. You won't be able to 'hot swap it'. ORMs are useful but they leak because of how complicated the problem they solve is and for plenty of performance reasons (Like Select n+1 for example).
Best Answer
Whenever you define a non-CRUD operation you will be in trouble to find a HTTP method, which describes the same thing. This can be solved by defining a new resource. So REST resources cannot be mapped 1:1 to entities. That's the problem in your case.
Imho you need a domain model which is working with the entities and you need a delivery method, which transforms the HTTP request into commands or queries addressed to your domain model. I would check domain driven design if I were you, maybe you learn something you can use in your project.