Wicket
  1. Wicket
  2. WICKET-4088

Make Application#init() run after IInitializers to allow the application to override any settings configured by initializers

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.4.18, 1.5.0
    • Fix Version/s: 6.0.0-beta1
    • Component/s: wicket
    • Labels:
      None

      Description

      wicket.properties urls are added to an HashSet, causing the IInitializers to be loaded in random order. In AbstractClassResolver.getResources, the HashSet should be changed to a LinkedHashSet.

        Activity

        Hide
        Igor Vaynberg added a comment -

        what is the usecase for requiring ordering?

        Show
        Igor Vaynberg added a comment - what is the usecase for requiring ordering?
        Hide
        Emond Papegaaij added a comment -

        It allows you to define wicket.properties files in you context, that will always be executed last.

        Show
        Emond Papegaaij added a comment - It allows you to define wicket.properties files in you context, that will always be executed last.
        Hide
        Martin Grigorov added a comment -

        It depends on the order that the class loader returns them. And I'm not sure whether there is specified order in the JEE specifications.

        Show
        Martin Grigorov added a comment - It depends on the order that the class loader returns them. And I'm not sure whether there is specified order in the JEE specifications.
        Hide
        Emond Papegaaij added a comment -

        AbstractClassResolver tries 3 different classloaders. Starting with the classloader for Application, followed by the classloader for your application, followed by the context classloader (or a custom classloader, if a different subclass of AbstractClassResolver is used). IMHO, the last should always be the most specific, and it's an ideal location to perform some additional configuration that depends on the configuration set by the other initializers.

        Show
        Emond Papegaaij added a comment - AbstractClassResolver tries 3 different classloaders. Starting with the classloader for Application, followed by the classloader for your application, followed by the context classloader (or a custom classloader, if a different subclass of AbstractClassResolver is used). IMHO, the last should always be the most specific, and it's an ideal location to perform some additional configuration that depends on the configuration set by the other initializers.
        Hide
        Igor Vaynberg added a comment -

        right. but what specifically? and if you are looking for something "last" why not put it into application's init()?

        i am rather wary of introducing any kind of ordering into this. iinitializers are meant to be standalone and do things orthogonal to other iinitializers. creating dependencies between them is a bad idea as they are not easily controlled since they rely on classpath ordering, etc.

        also introducing an ordering means introducing a reverse-ordering for idestroyers. unless i hear a very convincing usecase for this im -1.

        Show
        Igor Vaynberg added a comment - right. but what specifically? and if you are looking for something "last" why not put it into application's init()? i am rather wary of introducing any kind of ordering into this. iinitializers are meant to be standalone and do things orthogonal to other iinitializers. creating dependencies between them is a bad idea as they are not easily controlled since they rely on classpath ordering, etc. also introducing an ordering means introducing a reverse-ordering for idestroyers. unless i hear a very convincing usecase for this im -1.
        Hide
        Emond Papegaaij added a comment -

        We wanted to override a setting set by one of the initializers. init() is called before the initializers run, so it is not possible to do it there. initializeComponents() (the method that runs the initializers) is final, so can't do it there either. Which only leaves validateInit(), but this method is really not meant for that. I agree that you should not rely on the exact order in which initializers are loaded, but as I said, the context classloader always comes last. This at least gives you a chance to define your own initializer that runs after the others. As far as I'm concerned, that's the only usecase this is supposed to support.

        Of course, this could also be solved by another init method, which is called right after initializeComponents(). Perhaps a componentsInitialized() method?

        Show
        Emond Papegaaij added a comment - We wanted to override a setting set by one of the initializers. init() is called before the initializers run, so it is not possible to do it there. initializeComponents() (the method that runs the initializers) is final, so can't do it there either. Which only leaves validateInit(), but this method is really not meant for that. I agree that you should not rely on the exact order in which initializers are loaded, but as I said, the context classloader always comes last. This at least gives you a chance to define your own initializer that runs after the others. As far as I'm concerned, that's the only usecase this is supposed to support. Of course, this could also be solved by another init method, which is called right after initializeComponents(). Perhaps a componentsInitialized() method?
        Hide
        Igor Vaynberg added a comment -

        which initializer is causing you the headeache? all initializers do is call application.getsettings().set("foo") so why cant you do the same in your application#init and override whatever the initializer did?

        Show
        Igor Vaynberg added a comment - which initializer is causing you the headeache? all initializers do is call application.getsettings().set("foo") so why cant you do the same in your application#init and override whatever the initializer did?
        Hide
        Emond Papegaaij added a comment - - edited

        It is not that an initializer is causing problems. We want to set a custom header response decorator, based on the one WiQuery uses, but as I said, the application.init runs first. So if I set it in init, it will get overridden by the WiQuery initializer. You cannot override the initializers from application.init, it is the other way around. Perhaps, these methods should be swapped, but that is a breaking change, because the outcome may no longer be the same. For now, we've put the code in validateInit, but I consider that a hack.

        Show
        Emond Papegaaij added a comment - - edited It is not that an initializer is causing problems. We want to set a custom header response decorator, based on the one WiQuery uses, but as I said, the application.init runs first. So if I set it in init, it will get overridden by the WiQuery initializer. You cannot override the initializers from application.init, it is the other way around. Perhaps, these methods should be swapped, but that is a breaking change, because the outcome may no longer be the same. For now, we've put the code in validateInit, but I consider that a hack.
        Hide
        Martin Grigorov added a comment -

        I agree that application#init() should be the last executed.
        External library/plugin should set its preferences but the application should always have the final word.

        Show
        Martin Grigorov added a comment - I agree that application#init() should be the last executed. External library/plugin should set its preferences but the application should always have the final word.
        Hide
        Emond Papegaaij added a comment -

        But I don't it is possible to change that for wicket 1.5.2, because it may change the configuration of applications. For example, if application.init sets a() to true, and an initializer sets it to false, it will be false now, but it will become true if you switch the order.

        Show
        Emond Papegaaij added a comment - But I don't it is possible to change that for wicket 1.5.2, because it may change the configuration of applications. For example, if application.init sets a() to true, and an initializer sets it to false, it will be false now, but it will become true if you switch the order.
        Hide
        Igor Vaynberg added a comment -

        initializers are there to synchronize certain things between application nodes. for example, it gives libraries a way to set up shared resources, etc. they are not really there to "configure" the application. i consider the current case of wiquery setting something like that a misuse of the initializer. we will change the order in 1.6.0.

        Show
        Igor Vaynberg added a comment - initializers are there to synchronize certain things between application nodes. for example, it gives libraries a way to set up shared resources, etc. they are not really there to "configure" the application. i consider the current case of wiquery setting something like that a misuse of the initializer. we will change the order in 1.6.0.
        Hide
        Emond Papegaaij added a comment -

        It think this should be as simple as this patch

        Show
        Emond Papegaaij added a comment - It think this should be as simple as this patch

          People

          • Assignee:
            Martin Grigorov
            Reporter:
            Emond Papegaaij
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development