-
-
Notifications
You must be signed in to change notification settings - Fork 405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Di changes #1876
Di changes #1876
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@dazinator I think the culture/resource exception is masking the underlying issue, which is that the change broke method-level injection and is now throwing exceptions. So there are two issues - one is clearly a missing resource that I'm sure has been a hidden bug for some time. The other is the underlying issue that the tests are failing. You should be able to run at least the netcore tests locally on your dev machine. Some of the netfx tests rely on LocalDb and other things being installed, but the netcore tests (iirc) are all pure in-memory. |
I really don't think it will work to not provide a scoped provider btw. Without some scoped provider I don't see how most data access scenarios are viable, because things like database connections are normally scoped. In other words, though the current situation might be imperfect, there must be some scoped provider available to any code running on the server-side data portal right? |
The scoped provider needs to be set at the point where the scope is created. With my changes, csla will use this scoped provider if its set, if its not set it will fallback (either to checking for httpcontext, or the "default" application level service provider acquired on startup which is the root scope if you like). The bit I havent looked at yet, is - for a remote data portal, there should be an incoming request and and outgoing response for that data portal operation. That whole operation csla should wrap in its own DI scope (similar to what aspnet core does for each request) I.e on a remote data portal server, when a request is received csla should create a new DI scope using the Default service provider, then set that as the current scoped service provider in LocalContext |
All that is required is to add the English text to the non-localized resx file. All other languages will get that English text until folks update the localized resx files with translations. |
I see what you are saying. This may be something that needs to be done in each data portal host, because I suspect the rules may vary by host. In other words, if you are hosting in a console app or something, then it is up to you entirely. But if you are hosting in ASP.NET perhaps the runtime has already created a scope for you? |
Ok I'll make that resx change. Im typing on a mobile and accidentally hit the close pr button and then my battery died whilst mid way through editing my last comment! The joys of tech |
Exactly, as we know aspnet core does. |
.. and the reason I used the fallback logic (if there is no scoped IServiceProvider set in local context, then use the default IServiceProvider) is.. in cases where csla is used in a simple console app or unit test, the user might not feel the need to create any new DI scope at all. Technically csla doesn't impose any constraint on what sort of scope its being used in. If the DI system has a problem with scopes (say with EF) it will surface a DI exception which I think is valid. |
So my question remains: how do you get a reference to the scoped provider created by aspnet? |
Do you mean asp.net core? If so there are a couple of ways. If you mean legacy aspnet then I have no idea as I havent touched that in what feels like decades. In asp.net core, take a look at the context manager in this PR, if the user hasn't set a specific scoped sp, it will fall back to checking for an HttpContext (via IHttpContextAccessor) and then getting the scoped provider from HttpContext.RequestServices which is the sp scoped to current request. if there is no active httpcontext it falls back to the default sp. This saves the user (or csla) from having to have middleware that sets the scoped sp at the start of every request (which is the second way). The second way is as I posted on one of our recent discussions - you add middleware to the middleware pipeline that runs at the start of a request (csla could do this in its UseCsla() method that gets access to IApplicationBuilder). The middleware gets the current httpcontext so again, it can set the csla current scoped sp from HttpContext.RequestServices - but like I say technically the context provider does this check for you so doing this shouldn't be necessary if you are using the correct context manager for asp.net core. |
This comment has been minimized.
This comment has been minimized.
I think the reason you can't run tests is related to something with the vs test runner. I figured this out long ago, changed the setting, and then promptly forgot what I did 😳 In the test runner you need to change the default for AnyCPU assemblies so they run in x64 mode Then close and reopen the csla.test solution and the tests should run. I'm not hitting the resource issue when running the tests interactively, so I'm not sure why the build server is having that issue. But the tests absolutely are failing, so something in the PR broken method invocation. |
Ok thanks, I'll give that a go tomorrow! |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
You know, I think I had to delete the I don't know why, because the tests run in my normal working directory - but when I cloned your fork they wouldn't run for me either. Changing the test runner to x86 and deleting those files got them all running for me. |
Thanks - deleting those files, and setting test runner back to "Any Cpu" worked! |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
- Changes are made to Csla.Shared.Resources and resx files should be copied into project.
This comment has been minimized.
This comment has been minimized.
I'm tempted to remove the change I made to throw this exception resulting in the need for the resx entry - just to see if the build then passes - that change is somewhat unrelated anyway. |
@rockfordlhotka |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that new inject exception type should be removed as well?
Done |
I am using the 5.4.1 Preview & .Net 5.0 I am getting this strange error. Could not find the resource "Csla.Properties.Resources.resources" among the resources "" embedded in the assembly "Csla", nor among the resources in any satellite assemblies for the specified culture. Perhaps the resources were embedded with an incorrect name. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Reminder: Make sure to review CONTRIBUTING.MD and make sure you've sent in your signed contributor agreement.
Issue: #1874
This PR:
CSLA is no longer responsible for creating or disposing DI scopes.
CSLA is given a "default" IServiceProvider - which it will use as a "fallback" if there is not "scope" specific IServiceProvider set in LocalContext.
The aspnet core version of ContextManager , has an additional fallback which saves the user having to set the IServiceProvider for the current request, the precedence goes:
var currentSp = LocalContext ?? HttpContext.RequestServices ?? _defaultSp;
- same concept for the System.Web version.Because CSLA doesn't ever create a scope, it doesn't ever need to dispose a scope either.
NOTE: I have been having problems trying to get the tests to run on my local VS.. Seeing all sorts of weird behaviour. Happy to discuss but probably warrants a seperate issue. Will focus on getting appveyor tests to pass.
I throw an exception on @Inject with a null service provider - but I have not added a resx error message.. I am not fluent in many languages..I removed this additional change from this PR as adding resx entries was breaking the build for unknown reasons. This could be a seperate PR.