Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.2.0.GA
    • Fix Version/s: 3.3.0.CR1
    • Component/s: Core
    • Labels:

      Description

      NH 3.2 has a bug in the default proxy factory (not sure if it's been reported). The generated proxy assembly does not pass peverify because the proxy type constructor calls object::ctor instead of the base type (entity) constructor. This means that lazy loads are limited to full-trust policy.

      I fixed the proxy factory to emit call the "real" base type constructor, but this would have the side-effect of preventing constructor DI in entities. Then I added a new provider and contract (DefaultEntityInjector and IEntityInjector) so that DI is supported out-of-box by providing an implementation. The implementation simply needs to provide the constructor arguments for NH's call to Activator.CreateInstance.

      I added a test project that demonstrates default constructor, interface based proxy, and non-default constructor cats. Also, peverify reports zero errors on the dynamic assembly. This has also been tested in a pseudo-medium trust environment. The required reflection permissions above default medium are emit & protected member access.

      A pull request has requested from https://github.com/nhibernate/nhibernate-core/pull/2

        Issue Links

          Activity

          Hide
          Richard Brown added a comment -

          @Nikita - I'm not sure. Note, the fix didn't create a new feature, it just made the existing feature generate pe-verifiable code, so I'm not convinced there is a need for this. Perhaps it could be relaxed, but maybe NH should log a warning? A separate JIRA for the 'feature request' could be raised if it's wanted.

          @cremor, the virtual methods will be passed to the base class default implementation during construction (before the interceptor if defined), and should not cause any extra lazy-loading, but let us know if you spot anything going wrong.

          Show
          Richard Brown added a comment - @Nikita - I'm not sure. Note, the fix didn't create a new feature, it just made the existing feature generate pe-verifiable code, so I'm not convinced there is a need for this. Perhaps it could be relaxed, but maybe NH should log a warning? A separate JIRA for the 'feature request' could be raised if it's wanted. @cremor, the virtual methods will be passed to the base class default implementation during construction (before the interceptor if defined), and should not cause any extra lazy-loading, but let us know if you spot anything going wrong.
          Hide
          Randy Lee added a comment -

          @Richard - I think this is a good solution to the issue.

          Please read the following in the context of how I use NH. There may be other use-cases of NH that invalidate my assumptions...

          As for .ctor DI, I use NH with DDD, and there are times when injecting dependencies into the proxy objects created by NH make sense. In general, It is good practice to avoid dependencies in entity constructors for the simple reason that this hides the intention. However, sometimes you need to do it (i.e. messaging, validation, etc...) If this is the case, the re-constituted entities (proxies in some cases like a lazy-loaded many-to-one) need to be constructed with the dependencies intact.

          I also chose to remove the public constructor restriction in my bytecode provider. I prefer to have the aggregate root entity factory create instances of the the other entities in the aggregate. I honestly don't see a reason why public constructors are required in NH. It seems to detract from the POCO goal.

          Hopefully, the modified bytecode provider (linked above) can make it into core in the future as a new feature (in some variation). It has been used on several projects that I am aware of without any issues.

          Show
          Randy Lee added a comment - @Richard - I think this is a good solution to the issue. Please read the following in the context of how I use NH. There may be other use-cases of NH that invalidate my assumptions... As for .ctor DI, I use NH with DDD, and there are times when injecting dependencies into the proxy objects created by NH make sense. In general, It is good practice to avoid dependencies in entity constructors for the simple reason that this hides the intention. However, sometimes you need to do it (i.e. messaging, validation, etc...) If this is the case, the re-constituted entities (proxies in some cases like a lazy-loaded many-to-one) need to be constructed with the dependencies intact. I also chose to remove the public constructor restriction in my bytecode provider. I prefer to have the aggregate root entity factory create instances of the the other entities in the aggregate. I honestly don't see a reason why public constructors are required in NH. It seems to detract from the POCO goal. Hopefully, the modified bytecode provider (linked above) can make it into core in the future as a new feature (in some variation). It has been used on several projects that I am aware of without any issues.
          Hide
          Nikita Govorov added a comment -

          @Richard - I'm completely aware that the feature was introduced with built-in proxy factory(based on LinFu) of NH 3.2.
          I've created two improvements:
          https://nhibernate.jira.com/browse/NH-3029 - allows(actually does not reckon it as invalid for trusted environments) creation of proxies without based on objects default non-private ctor.
          https://nhibernate.jira.com/browse/NH-3030 - perform materialization of entities and components without default non-private ctor.

          Show
          Nikita Govorov added a comment - @Richard - I'm completely aware that the feature was introduced with built-in proxy factory(based on LinFu) of NH 3.2. I've created two improvements: https://nhibernate.jira.com/browse/NH-3029 - allows(actually does not reckon it as invalid for trusted environments) creation of proxies without based on objects default non-private ctor. https://nhibernate.jira.com/browse/NH-3030 - perform materialization of entities and components without default non-private ctor.
          Hide
          Richard Brown added a comment -

          @Randy, I think you might be confusing DI on the proxies with DI on the entities. I cannot think of a reason to have dependencies injected into the proxy (since they will never be accessed). All calls on the proxy are forwarded to the 'real' entity (which may need dependencies injected).

          @Nikita, Thanks for that. Specifically relying on the SkipVerification permission looks like a good suggestion.

          Show
          Richard Brown added a comment - @Randy, I think you might be confusing DI on the proxies with DI on the entities. I cannot think of a reason to have dependencies injected into the proxy (since they will never be accessed). All calls on the proxy are forwarded to the 'real' entity (which may need dependencies injected). @Nikita, Thanks for that. Specifically relying on the SkipVerification permission looks like a good suggestion.
          Hide
          Oskar Berggren added a comment -

          Closing issues fixed in 3.3.1.CR1.

          Show
          Oskar Berggren added a comment - Closing issues fixed in 3.3.1.CR1.

            People

            • Assignee:
              Richard Brown
              Reporter:
              Richard
            • Votes:
              20 Vote for this issue
              Watchers:
              13 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Who's Looking?