Skip to content
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

#isolate: broken #553

Open
GoogleCodeExporter opened this issue Mar 25, 2015 · 23 comments
Open

#isolate: broken #553

GoogleCodeExporter opened this issue Mar 25, 2015 · 23 comments

Comments

@GoogleCodeExporter
Copy link

The problem is that #isolate: depends on WASession's filter state being
restored. Unfortunately, the state is restored by the saved Continuation,
which isn't looked up until #handleFiltered: (after we have already been
through the *current* chain of filters).

The obvious solution would seem to involve applying the continuation state
in #handle: and then invoking the continuation in #handleFiltered: but this
means (a) exposing the restoring of continuation state outside
WAContinuation (and splitting the restore/invoke into two operations) and
(b) looking up the continuation in two places. Neither of those seem very
nice. Also, are we certain that it even makes sense to revert the filter
chain for all filters?

The ugliness of this issue is pointing to a design problem somewhere in my
mind...

Original issue reported on code.google.com by [email protected] on 28 Feb 2010 at 6:12

@GoogleCodeExporter
Copy link
Author

[deleted comment]

@GoogleCodeExporter
Copy link
Author

On Mon, Jul 19, 2010 at 11:27 AM, Boris Popov, DeepCove Labs (SNN) 
> I’m trying to figure out if there’s a suggested fix out there or if I 
should
> be redesigning my isolate: blocks on the task to restore intended
> functionality?

I think it needs discussion.

Looking at it now, it used to be implemented with a Decoration but is now a 
RequestFilter. The reason for this is that we simplified the callback 
processing so it was handled centrally rather than delegating it down the 
Component tree. Thus, a Decoration no longer has the opportunity to prevent the 
execution of callbacks.

The semantics, however, are different because the isolation now applies to the 
whole page, rather than a single component (though the old implementation did 
trigger a page expired response, so I guess the end result wasn't *that* 
different).

Thinking about it again now, this probably means *all* senders of 
#filterWith:during: are broken. The semantics of affecting a whole page by 
calling  wrapping method on a Component are a bit wonky, though, and I really 
am not convinced we want to start backtracking filters. 

It's a shame that we lost the nested control over callbacks but I think we need 
to understand how important #isolate: and #authenticateWith:during: are to 
users. I can think of a few directions to go in trying to solve this but 
they're all pretty ugly.

Original comment by [email protected] on 19 Jul 2010 at 8:31

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

FYI: Filters are backtracked in Sessions, not in other request handlers.

Original comment by renggli on 19 Jul 2010 at 9:07

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Hrm... so they are. Ok, I got myself confused.

So the bug was as I stated originally: there's little point in backtracking the 
filters on Session when the filters have already been used by the time the 
state is restored. I think I remember the design problem I was thinking about 
but I'm not about to start mucking with design in 3.0 at this point.

So the question is, do we remove #isolate: for now or do we need a temporary, 
awful, nasty hack of some kind? (waiting for response on the mailing list as to 
how many people use the bloody thing)

Original comment by [email protected] on 19 Jul 2010 at 9:35

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

To me it looks like we need to let the session restore it's state in 
application before we delegate to it. Or am I missing something?

Original comment by [email protected] on 20 Jul 2010 at 5:04

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

That's more or less what I'm saying. I think we delegate too much 
responsibility to the session somehow. But the session doesn't know how to look 
up the correct continuation until it is given the request.

There's something funny about how the sessions continuations work still (I 
alluded to this at the last Zug sprint). This was improved somewhat by removing 
the WARenderLoop class but somehow there's an sense of the continuations being 
both within the Session and around it, and that's a bit funny.

I think we can rethink this a bit and get it working but not now. In the 
meantime, sure, we can restore the state in either #handle: or in the 
application but that has the negatives of (a) and (b) as I mentioned in the 
original report... and I call that a hack.

Original comment by [email protected] on 20 Jul 2010 at 8:21

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

I'd prefer a working hack, to a non-working ideal solution.

Original comment by [email protected] on 20 Jul 2010 at 10:20

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

I'm with Philippe on this one, I don't care how clean the internal solution is 
so long as #isolate: functionality works as advertised. Granted a flow could be 
implemented without it, but it does take away from the simplicity of wrapping 
critical code with the isolate: block.

Original comment by [email protected] on 20 Jul 2010 at 11:07

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Issue 486 has been merged into this issue.

Original comment by renggli on 20 Jul 2010 at 12:16

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

"I'd prefer a working hack, to a non-working ideal solution."

Sure, but I'd also prefer the removal of a rarely used non-working feature (if 
that's what it is) to the addition of a rarely used working hack.

We can't make an informed decision until we know how important this is to how 
many people.  If only a few existing users want this, it may be better to find 
them a workaround and remove the feature.

If we *are* keeping the feature, we need to consider what the desired semantics 
actually are and how it can be implemented in a way that fits with the current 
design. #isolate: was implemented back in the days when everything was 
continuations, there were no decorations, no filters, etc. It's been kind of 
hacked onto each new way of doing things from version to version.

So can we please take the heat out of this discussion and simply focus on who 
uses this, how they use it, and what they'd like the semantics to be?

Original comment by [email protected] on 20 Jul 2010 at 6:45

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

So, looking at semantics, is it even/always the desired behaviour that the 
whole page gets expired when you click on a link inside an isolated component?

Let's say it is desired. In previous versions, only links rendered inside the 
component that was using isolation would trigger the expiry. Even if the 
current implementation was working, the use of a filter instead of a decoration 
means it would now cause *any* link on the entire page to trigger the expiry 
page, no? Is that even good enough? Do you really want an expiry page if you 
click a navigation link?

Original comment by [email protected] on 20 Jul 2010 at 6:51

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

@Comment 11
For me that would be OK

Original comment by [email protected] on 21 Jul 2010 at 1:42

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

@Comment 12

Ditto.

Original comment by [email protected] on 21 Jul 2010 at 2:09

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

I've removed #isolate: from the 3.0 final build (better to be missing than 
there and broken) and put it back as is in the 3.1 repository. Fixes should be 
made there.

Original comment by [email protected] on 8 Aug 2010 at 9:57

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Any progress on this? I'm planning to build an e-shop soon, and isolate is 
pretty important for checkout and things like that.

Original comment by [email protected] on 23 Mar 2011 at 10:48

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Nope, sorry. We can't give you a time frame as well.

Original comment by [email protected] on 24 Mar 2011 at 3:09

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Any progress?? I'm also building a web app where this would be an important 
capability.

Original comment by [email protected] on 15 Aug 2012 at 9:44

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

No, sorry.

Original comment by [email protected] on 16 Aug 2012 at 7:26

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Any progress on this?

Original comment by [email protected] on 20 Oct 2014 at 1:30

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

No. sorry.

Original comment by [email protected] on 21 Oct 2014 at 5:22

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Well that's really exciting. I probably only have another thirty years to
live.

Original comment by [email protected] on 21 Oct 2014 at 10:16

  • Added labels: ****
  • Removed labels: ****

@recursive68
Copy link

Any news on this ? If it is never going to be fixed how do I update code that currently uses isolate ?

Thanks

@recursive68
Copy link

I'm guessing from the lack of response this issue is never going to be fixed. I think this is real shame. I guess I will have to look at other web frameworks in that case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants