Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None
    • Other Info:
      Patch available
    • Affects version (Component):
      Servlet Service Framework

      Description

      When a servlet service request is created, parameters from the parent request are ignored. This means that the sub request is performed as a fresh and clean new call. This would avoid any possible side-effects, but is very inconvenient in practice because you don't even know the request header parameters from the original (external) request. Additionally you can only pass information which is part of the returned stream, which is e.g. a blocker to use the servlet protocol together with the control flow implementations. Those make use of special request parameters to transport the model ("bizdata") to the view layer.

        Issue Links

          Activity

          Reinhard Poetz made changes -
          Resolution Fixed [ 1 ]
          Status Open [ 1 ] Closed [ 6 ]
          Hide
          Reinhard Poetz added a comment -
          patch applied. req params & attrs and session are passed/shared.
          Show
          Reinhard Poetz added a comment - patch applied. req params & attrs and session are passed/shared.
          Hide
          Reinhard Poetz added a comment -
          Thanks for the update on your patch. I had already been wondering how your code could work correctly with only one getNames() implementation ;-)

          I've applied the patch locally and it looks good so far. I will commit it some time next week. Then I will work on setting up a test infrastructure so that we can test all this really important code thoroughly.
          Show
          Reinhard Poetz added a comment - Thanks for the update on your patch. I had already been wondering how your code could work correctly with only one getNames() implementation ;-) I've applied the patch locally and it looks good so far. I will commit it some time next week. Then I will work on setting up a test infrastructure so that we can test all this really important code thoroughly.
          Hide
          Rice Yeh added a comment -
          getValues() is used to implement ServletRequest.getParameterMap() which I define it to return all parameters including passed to the current request's caller and the caller's caller,...etc.

          I have move getValues() to Paramerters class because it is just used by Parameters. Also in the original implementation, getNames() always include native parameter's names no matter it is in a class Attributes or Header. In Attributes, it should include attribute's names. In Headers, it should include header's names.

          Your approach could fulfill the same purpose since eventually it iterates all call frames by recursive way. I just feel iteration is easier for me in implementing overriding.

          Regards,
          Rice

          Show
          Rice Yeh added a comment - getValues() is used to implement ServletRequest.getParameterMap() which I define it to return all parameters including passed to the current request's caller and the caller's caller,...etc. I have move getValues() to Paramerters class because it is just used by Parameters. Also in the original implementation, getNames() always include native parameter's names no matter it is in a class Attributes or Header. In Attributes, it should include attribute's names. In Headers, it should include header's names. Your approach could fulfill the same purpose since eventually it iterates all call frames by recursive way. I just feel iteration is easier for me in implementing overriding. Regards, Rice
          Hide
          Grzegorz Kossakowski added a comment -
          > But 'parentRequest' used in the code seems a little weird, my original version callingRequest or just caller might be better because we
          > might get confused with the parent used in block inheritance.

          Agreed.

          > By the way, in the above post it is stated that there is some controversial discussion in applying the idea used in this patch on session. What is it?

          Have a look at this thread:
          http://thread.gmane.org/gmane.text.xml.cocoon.cvs/25924/focus=76076

          It's best to read whole discussion because lots of great thoughts has been presented.

          > After a more detail look at the code, I think that iteration over call stack is necessary in implementing getValues() and getNames() method.
          > For Object getValue(String name), recursive call is enough. But for getting all values or all names for the whole values in the calling chain,
          > iterating each call frame to collect their values is necessary. However, by implementing these 2 methods (getValues() and getNames()),
          > then there is no such thing as values hiding.

          Could you explain in detail why it's needed for getValues()? Why approach outlined in my above comment won't work?

          > Also I find some bug in my previous patch. So I have the new implementation attached for reference.

          Could you be more specific? What bug you have discovered?
          Show
          Grzegorz Kossakowski added a comment - > But 'parentRequest' used in the code seems a little weird, my original version callingRequest or just caller might be better because we > might get confused with the parent used in block inheritance. Agreed. > By the way, in the above post it is stated that there is some controversial discussion in applying the idea used in this patch on session. What is it? Have a look at this thread: http://thread.gmane.org/gmane.text.xml.cocoon.cvs/25924/focus=76076 It's best to read whole discussion because lots of great thoughts has been presented. > After a more detail look at the code, I think that iteration over call stack is necessary in implementing getValues() and getNames() method. > For Object getValue(String name), recursive call is enough. But for getting all values or all names for the whole values in the calling chain, > iterating each call frame to collect their values is necessary. However, by implementing these 2 methods (getValues() and getNames()), > then there is no such thing as values hiding. Could you explain in detail why it's needed for getValues()? Why approach outlined in my above comment won't work? > Also I find some bug in my previous patch. So I have the new implementation attached for reference. Could you be more specific? What bug you have discovered?
          Rice Yeh made changes -
          Attachment BlockCallHttpServletRequest.patch [ 12372298 ]
          Hide
          Rice Yeh added a comment -
          Hi,
            After a more detail look at the code, I think that iteration over call stack is necessary in implementing getValues() and getNames() method. For Object getValue(String name), recursive call is enough. But for getting all values or all names for the whole values in the calling chain, iterating each call frame to collect their values is necessary. However, by implementing these 2 methods (getValues() and getNames()), then there is no such thing as values hiding.

            Also I find some bug in my previous patch. So I have the new implementation attached for reference.

          Regards,
          Rice
          Show
          Rice Yeh added a comment - Hi,   After a more detail look at the code, I think that iteration over call stack is necessary in implementing getValues() and getNames() method. For Object getValue(String name), recursive call is enough. But for getting all values or all names for the whole values in the calling chain, iterating each call frame to collect their values is necessary. However, by implementing these 2 methods (getValues() and getNames()), then there is no such thing as values hiding.   Also I find some bug in my previous patch. So I have the new implementation attached for reference. Regards, Rice
          Hide
          Rice Yeh added a comment -
          By the way, in the above post it is stated that there is some controversial discussion in applying the idea used in this patch on session. What is it?

          Regards,
          Rice
          Show
          Rice Yeh added a comment - By the way, in the above post it is stated that there is some controversial discussion in applying the idea used in this patch on session. What is it? Regards, Rice
          Hide
          Rice Yeh added a comment -
          Agree. But 'parentRequest' used in the code seems a little weird, my original version callingRequest or just caller might be better because we might get confused with the parent used in block inheritance.

          Regards,
          Rice

          Show
          Rice Yeh added a comment - Agree. But 'parentRequest' used in the code seems a little weird, my original version callingRequest or just caller might be better because we might get confused with the parent used in block inheritance. Regards, Rice
          Hide
          Grzegorz Kossakowski added a comment -
          Let's suppose you have these three requests on the CallStack:
          r3
          r2
          r1

          Where r1 is the first one created and has no caller request associated with, r2 has r1 as caller and r3 has r2 as caller. Then if you call getValus() method of r3 it will return list consisting of items obtained from r2.getValues() call merged with list of values of r3 itself. If there is a conflict r3 overrides, of course. You should notice, that thanks to the fact r2 has a r1 as caller r2 will ask r1 for values and merge with its own. As you see we get essentially the same result as we would traverse whole callstack.

          This approach is better for even more than just clean implementation. You may happen to don't want request parameters to be passed to the called servlet. Then it's damn easy to achieve, you just need to use modified version of BlockCallHttpServletRequest that will not care about caller. With your approach it is not possible.

          I hope that it's clear now and we don't overlook anything so we can commit this very precious functionality.
          Show
          Grzegorz Kossakowski added a comment - Let's suppose you have these three requests on the CallStack: r3 r2 r1 Where r1 is the first one created and has no caller request associated with, r2 has r1 as caller and r3 has r2 as caller. Then if you call getValus() method of r3 it will return list consisting of items obtained from r2.getValues() call merged with list of values of r3 itself. If there is a conflict r3 overrides, of course. You should notice, that thanks to the fact r2 has a r1 as caller r2 will ask r1 for values and merge with its own. As you see we get essentially the same result as we would traverse whole callstack. This approach is better for even more than just clean implementation. You may happen to don't want request parameters to be passed to the called servlet. Then it's damn easy to achieve, you just need to use modified version of BlockCallHttpServletRequest that will not care about caller. With your approach it is not possible. I hope that it's clear now and we don't overlook anything so we can commit this very precious functionality.
          Hide
          Rice Yeh added a comment -
          In my mind, iterating the call stack form the first call frame to the last call frame to put values from each frame in a map ( which has value's name as key) does have value from the later call frame over-write the previous frame's value in the map. It is just so nature. So be honest, i do not really understand your question. But, will just passing parameters from caller (you refer it as 'parent') satisfy the overriding nature I mentioned in http://www.mail-archive.com/dev@cocoon.apache.org/msg52250.html? If yes, then go with your way, I might not really understand the code in servlet service. Or we just have different concept on values' overriding.

          In the following, i recite my example in http://www.mail-archive.com/dev@cocoon.apache.org/msg52250.html.

          "Attribute values (here attribute refers to the general meaning, it includes the parameters,
          headers and attributes in request and the attributes in session and context)
          in s2 can override values in s1. For example the value of an attribute a1 is
          vs1 in s1, when s1 call s2, s2 might have attribute a1 with value vs2
          because a1 is overridden (here, I am not sure whether it is correct to use
          the word 'override'). However, for another block s3 called by s1 which does
          not override a1, the value of a1 is still vs1"

          Regards,
          Rice

          Show
          Rice Yeh added a comment - In my mind, iterating the call stack form the first call frame to the last call frame to put values from each frame in a map ( which has value's name as key) does have value from the later call frame over-write the previous frame's value in the map. It is just so nature. So be honest, i do not really understand your question. But, will just passing parameters from caller (you refer it as 'parent') satisfy the overriding nature I mentioned in http://www.mail-archive.com/dev@cocoon.apache.org/msg52250.html? If yes, then go with your way, I might not really understand the code in servlet service. Or we just have different concept on values' overriding. In the following, i recite my example in http://www.mail-archive.com/dev@cocoon.apache.org/msg52250.html . "Attribute values (here attribute refers to the general meaning, it includes the parameters, headers and attributes in request and the attributes in session and context) in s2 can override values in s1. For example the value of an attribute a1 is vs1 in s1, when s1 call s2, s2 might have attribute a1 with value vs2 because a1 is overridden (here, I am not sure whether it is correct to use the word 'override'). However, for another block s3 called by s1 which does not override a1, the value of a1 is still vs1" Regards, Rice
          Reinhard Poetz made changes -
          Other Info [Patch available]
          Reinhard Poetz made changes -
          Summary Finish blocks-fw implementation Passing parameters to sub calls
          Affects version (Component) Parent values: Servlet Service Framework(10175).
          Description When a servlet service request is created, parameters from the parent request are ignored. This means that the sub request is performed as a fresh and clean new call. This would avoid any possible side-effects, but is very inconvenient in practice because you don't even know the request header parameters from the original (external) request. Additionally you can only pass information which is part of the returned stream, which is e.g. a blocker to use the servlet protocol together with the control flow implementations. Those make use of special request parameters to transport the model ("bizdata") to the view layer.
          Hide
          Reinhard Poetz added a comment -
          yes, I understand and agree. But Grek's and my question is why do you iterate the whole callstack? Wouldn't it be enough to pass parameters from the parent only?
          Show
          Reinhard Poetz added a comment - yes, I understand and agree. But Grek's and my question is why do you iterate the whole callstack? Wouldn't it be enough to pass parameters from the parent only?
          Hide
          Rice Yeh added a comment -
          Iteration over the whole call stack is my way to implement 'overriding' effect in values like parameters. For values' overriding, please see http://www.mail-archive.com/dev@cocoon.apache.org/msg52250.html.
          Show
          Rice Yeh added a comment - Iteration over the whole call stack is my way to implement 'overriding' effect in values like parameters. For values' overriding, please see http://www.mail-archive.com/dev@cocoon.apache.org/msg52250.html .
          Hide
          Grzegorz Kossakowski added a comment -
          I have taken a look on fragments where iteration of CallStack occurs and I'm almost sure it is not needed. Personally speaking I have no idea why it was done this way.

          I hope that Rice can give us an explanation. For now I would suggest to apply this patch _without_ iterating whole CallStack and only limit ourselves to calling request object.
          Show
          Grzegorz Kossakowski added a comment - I have taken a look on fragments where iteration of CallStack occurs and I'm almost sure it is not needed. Personally speaking I have no idea why it was done this way. I hope that Rice can give us an explanation. For now I would suggest to apply this patch _without_ iterating whole CallStack and only limit ourselves to calling request object.
          Reinhard Poetz made changes -
          Assignee Grzegorz Kossakowski [ grek ] Reinhard Poetz [ reinhard@apache.org ]
          Hide
          Reinhard Poetz added a comment -
          I've applied the patch except the part that deals with accessing the session because I remember some controversial discussions about it. I've brought up this topic on dev@cocoon and wait for the outcome of the discussion.
          Show
          Reinhard Poetz added a comment - I've applied the patch except the part that deals with accessing the session because I remember some controversial discussions about it. I've brought up this topic on dev@cocoon and wait for the outcome of the discussion.
          Hide
          Grzegorz Kossakowski added a comment -
          Thanks Rice for attaching this patch. I will review and commit it after upcoming next release of servlet-service-fw is made.
          Show
          Grzegorz Kossakowski added a comment - Thanks Rice for attaching this patch. I will review and commit it after upcoming next release of servlet-service-fw is made.
          Grzegorz Kossakowski made changes -
          Link This issue is part of COCOON-2132 [ COCOON-2132 ]
          Grzegorz Kossakowski made changes -
          Assignee Grzegorz Kossakowski [ grek ]
          Rice Yeh made changes -
          Attachment cocoon-servlet-service-impl.patch [ 12364983 ]
          Hide
          Rice Yeh added a comment -
          The previous patch does not include the modification in ServletServiceContextTestCase that is subject to the change in BlockCallHttpServletRequest. Attached patch has this modification included.

          Rice
          Show
          Rice Yeh added a comment - The previous patch does not include the modification in ServletServiceContextTestCase that is subject to the change in BlockCallHttpServletRequest. Attached patch has this modification included. Rice
          Rice Yeh made changes -
          Field Original Value New Value
          Attachment cocoon-servlet-service-impl.patch [ 12364973 ]
          Hide
          Rice Yeh added a comment -
          I think that Interblock communication mechanism is a must prerequisite to close this bug. see http://www.mail-archive.com/dev@cocoon.apache.org/msg52252.html.

          Attached is a patch for interblock communication. It works for me and one user who used this patch attached in cocoon-2066, which does not include attributes support in session. see http://www.mail-archive.com/users@cocoon.apache.org/msg39396.html

          Rice
          Show
          Rice Yeh added a comment - I think that Interblock communication mechanism is a must prerequisite to close this bug. see http://www.mail-archive.com/dev@cocoon.apache.org/msg52252.html . Attached is a patch for interblock communication. It works for me and one user who used this patch attached in cocoon-2066, which does not include attributes support in session. see http://www.mail-archive.com/users@cocoon.apache.org/msg39396.html Rice
          Hide
          Grzegorz Kossakowski added a comment -
          I think current version of servlet-service-fw is quite satisfying when it comes to handling Cocoon blocks.

          Could this issues be closed?
          Show
          Grzegorz Kossakowski added a comment - I think current version of servlet-service-fw is quite satisfying when it comes to handling Cocoon blocks. Could this issues be closed?
          Reinhard Poetz created issue -

            People

            • Assignee:
              Reinhard Poetz
              Reporter:
              Reinhard Poetz
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development