Make the WSCCI stack a singleton

You just reverted what was here before.

In term of design you just conscientiously wrote a singleton/static factory methods into the business object class. You could have, at least, moved them in another class.

Ok, it removes code, but what is the benefit of that except making the API less usable for potential contrib business edge cases?

EDIT: Having a non stack dependent context within the API allows modules to use their own contextes for business operations without messing with core, and without the need of extending it, which was my point originally. Maybe injecting the stack was not the best idea, but definitely have a way to use contextes without the stack is something I will not give up, for many reasons.

The whole goal was to make the stack optional, possibility of having multiple is a side effect, not the wanted feature. In the begining, Crell's goal was to get rid of all static properties of the API, the optional stack was also solving this.

I agree the stack injection was not the best way, but now that you hardcoded the static stack handling in Tracker, the API is unflexible yet again.

Succint or not, you have to see the future coming too. A single business piece of API must be either procedural, either OOP, but you cannot mix the two it makes the lot unreadable, and it spreads the code between namespaces and global namespace, and it spreads the code into files: it's a huge Drupal WTF.

I understand the framework is historically procedural, and I'm not fighting against, but if a piece of API is OOP, the only procedural function we want to see in it is only a wrapper for existing procedural code: the OOP code must not be coupled to such procedural code for code readability and maintainance purposes.

The drupal_get_context() was a global accessor, and the API was decoupled of it.

Putting back the static stack as you describe in #1 would desync the implementation from the interface, and design by contract wouldn't be true anymore: the interface has to be removed if you implement in a such way because internals then will be hardcoded in the Context implementation without the interface having the knowledge of it: providing a custom implementation is therefore non possible.

EDIT: OOP code is not a goal itself, but we get to the norm vs standard debate again: OOP comes with standards and paradigms, and using procedural code that manipulate global state/static objects inside the OOP encapsulated API breaks a shitload of them. Let's not confuse people used to code with it. Anyone can read and write procedural code pretty much as anyone can read and write OOP code, as long as it meet the standards, general habits and well accepted paradigms. Let's not reinvent the wheel again in Drupal.

It sounds a better approach for code readability, I guess, at least it encapsulate the static code and separate it from the rest.

If you are really looking into removing this stack, that's probably what I'd prefer. But instead of having static properties I'd go into a real singleton instead: at least if we want to change it later we just have to remove the static self reference instead of having to refactor all arround.

EDIT: My point is to decouple totally this static code from the Context class, if possible, it's easier to extend and refactor if needed then.

What's the point of an object that holds just the head pointer but doesn't maintain a stack? All of those objects are still there anyway.

I suppose yes a head pointer is all we need, as effectively it's just a linked list. I'm half tempted to try SplLinkedList, but it doesn't support the "cut off everything from this point on up" operation we need as far as I'm aware.

And db_select() is purely a wrapper in DBTNG. It is never called internally from within a class. I was very firm on that point when we were developing it. :-)

Crell +1

You don't directly access them either way. My point is that they're still in memory, so there's no memory savings.

Actually I just realized something. Without a stack, how do we do the "kill this object and everything above it" logic in O(1) time? Would only maintaining a head pointer let us do that?

Both solutions are logically good and work fine, but chx's one re-introduce direct procedural API calls from within the OOP API, whereas the procedural code is only a wrapper between OOP new code and procedural legacy code and should not be invasive over the OOP API.

In a component oriented logic, the components should be free of this kind of invasive code, except in specific implementations (i.e. the handlers, which are not part of the core but specific implementations that are supposed to be injected by other subsystems).

EDIT: I'm not being a pattern nazi here, but the Context API has been thought this way from the begining and this single one line procedural call undermine the whole design.

Per catch, and blessed by Larry, moving this and all other WSCCI issues to the Drupal core queue.

The notion of multiple stacks seems like a foolish and unnecessary complexity to me. I am certainty in agreement with chx that this should not go into core as-is.

I might rather see the accessor as a class method rather than a bare function, but that's a minor quibble over implementation.

I have heard zero real arguments so far, you guys realize that? OOP vs procedural is a false dichotomy

Like I said before, it's not procedural vs. OOP and never has been, but once you started an API in one of those, you must stick to it for code readability reasons. Mixing in the same API procedural and OOP is spagetti code, with high probably to end up unmaintable. Don't try to change the interpretation of what I said ealier it's YOU that always bring back this debate on.

I might rather see the accessor as a class method rather than a bare function, but that's a minor quibble over implementation.

Yes, if I heard all the complains about the multiple stack feature, like I said it wasn't a feature by itself but a side effect of another bug resolution. It was quite elegant, but it seems that most people don't understand why it was useful, so I happily agree to come back on it if it makes people happy too.

My proposal to double back about this would be something quite like has done chx, but into a singleton instance instead. The stack should only be manipulated via the Tracker instance because Context doesn't have to know it: a potential contrib developer could do its own stack if he needs to by extending the Tracker instance, which is a lot smaller than Context itself, so it's definitely easier.

@chx seems like a good compromise to me.

In order to restore the "context tree without no stack" feature, that contrib definitely could use in business code, I'd like to find a solution to make the Tracker class swappable.

How exactly is a class with all-static methods not a global, which we're trying to avoid? And why is that better than the drupal_get_context() function? The signature to call it from legacy code doesn't seem any nicer.

On the other hand, we do that [mixing OOP and procedural] all the time. Wrappers, factories etc.

If we do, it's a bug. Generally we have a thin procedural veneer over an entirely-OO base for BC reasons. That's different than mixing OO and procedural code together willy-nilly.

Like I was saying this morning, the feature I attempted to achieve in the beginning was the ability to use contextes outside of core stack, which seems a potential use case for contrib. And I won't seat on this particular use case since I already got it in working code of mine.

After speaking with chx, I'd say using a singleton for the core stack is doable (as I said, singleton, not a bunch of static methods on a pure static class), even if I'd prefer to inject some kind of stack object somewhere instead of using *any* static stuff (static is evil, we are trying to get rid of global state and static objects).

At least the singleton carries an instance, which is easier for a potential later refactor if we need to get rid of the static stuff (no modifications on the stack object itself apart removing the static getInstance() accessor). It's the easiest and smartest compromise I can see that actually fit with chx's rejects about this stack complexity.

Further, to implement the feature of "no stack context trees" I'd say we have to fully move stack handling into Tracker, and chx's #27 patch does it good, let's keep this it sounds wonderful to me. But in order to achieve the feature, we also need to get a dynamic Tracker class usage, so here what I'd propose:

• Create a TrackerInterface (for fun, at least it expose a clean component definition for the outside world
• Create a DefaultTracker (or VoidTracker) that does not handle any stack (that achieves the feature), and use this class per default in Context::lock() method.
• Now that this API is totally contextless and environment agnostic (it's a real decoupled component) let's create the CoreStackTracker implementation, that actually manipulates the stack singleton.
• Figure out a way of configuring the "tracker class to use" property in Context class, and let each addLaer() call gracefully propagate this property to children (we may need to have a ContextInterface::setTrackerClass() method in order to achieve this and being compatible with Context::addLayer() method that actually relies on ContextInterface)
• Once done that, just spawn the Drupal core root context with the right Tracker class property and rule the world.

All of this sounds complicated, but once we have a ContextInterface, and because we rely on it on various other generic operations, we have to impact it as soon as we make this kind of design change. This is the mandatory trade-off of the API in its actual state.

Anyone to throw a rock at me or does this sounds good to all the people that believe that the optional stack was too complex?

EDIT: Removed useless statements.
EDIT(2): Various typo.

EDIT: BTW I don't see any good argument in favor of suppressing parent handling functions.

missing

Focusing! ... on this issue is indeed missing what's important. You should focus on the rest of the API that highly focus itself on core's need. You are someone that highly knows how core works and you'd be highly valuable when it comes to plugging existing API's into this one.

target

Contrib! Target is contrib. Core itself never needed this kind of context, but most of the well know modules actually are struggling with contextual information (Panels, CTools, Views, Context, Rules, etc...). Core will become a target once it's own API will have matured a bit more.

provide

That's what we're doing since some monthes ago. Under the lead of Crell which may create code that actually diverge a lot of actual core habbits; That's for a greater good anyway because all this initiative doesn't include only this, but this as a the first piece of a greater puzzle where it all has a logic. Even if I do not agree with Crell about everything, I must admit I see in there planification and focus.

centralized

It is, if anyone finds the right tool for its use case.

win

Loosing. Time is what we're loosing with this issue.

design

Is the most important thing in order to get durable product.

work

Working? Yes it was! Still is, even before you read the code of it.

asks

Has done. Still working on my development box though.

complex

What is not? By the way, you're fighting to remove 10 lines. This stack injection was point to point similar to any other component injection, it's highly comprensible for most people.

unnecessary

Fighting over something that actually works is.

whatever

Exactly the word I had in mind, five minutes ago.

functionality

Is what you try to remove here.

crazy

The guy who wrote the actual Field API probably is.

edge

Business. Business is the word you were looking for. Any contrib module has a business of its own, every module owner is specialiazed in a business, any business needs adapted tools, and not to adapt itself to these tools. Else, we're falling under the law of instrument where the tool justify the mean.

case

Is indeed what you're making of me. Great thanks dude.

drupal_get_context

Will live a long life, whatever happens, because the whole goal of having a procedural wrapper is to ensure we won't let anyone fall behind.

EDIT: I don't really know what else I could answer to your post. In the Context API in which I participate (or at least try) in a positive manner and under a behavior directed by compromise, there is a huge piece of deeper and a lot more obscure code, such as the cache problems, partly solved using a weird handler inheritance alogrithm, followed by session handling refactoring follow-up, and a traversal logic so hard to document that documentation itself has its own bugs. Without saying language integration and path refactor are also very active issues with a lot people contributing.

Stop! Stop. Let's not let this escalate further. You are both smart people who want the best for Drupal.

What chx is saying is "Let's worry about getting the context system in first, then punt on the more advanced stuff to later, if we need it." However, he's doing so in a really condescending way that attacks the work the WSCCI people are doing. Knock it off.

Pounard is saying "While core doesn't have a direct use case currently, contrib is going to need this, and we've already solved it so why should we take it out?" However, he's doing so in a really condescending way that attacks the work the core people are doing. Knock it off.

The issue queue is not the place for this kind of personal attack shenanigans. Please stick to focused, technical arguments.

@webchick Thanks, I guess we all try to defend our opinion. My guess is that a lot of stuff really need work, so let's first solve the blocking problems before.

Right, so backing up...

Things we want:

1) Override/stacking ability. There are many possible uses here, but one we absolutely know about is the super-block style layout that is on the WSCCI roadmap. We're looking ahead to that.

2) Minimize/avoid globals and statics. They are architecturally sloppy and a bear trap, and make unit testing harder.

3) As few moving parts as is feasible given the above requirements.

The code in the wscci branch right now meets that fairly well. It's overridable, and there's only one static anywhere, in drupal_get_context(), which is only there to support legacy procedural code. The only complication is that the stack/tracker/context objects are more inter-related than we'd like. That's not a fatal problem, but if we can tidy that up we should.

Strictly speaking, whether we implement the "get the active context object from anywhere" using an actual stack, an array, or just a head pointer doesn't really matter. What matters is that we can override the context object and still support the "get active context" operation.

Maintaining multiple stacks is not a necessary feature per se; however, the ability to do so is a natural side-effect of point 2 above, and a good indication that we have succeeded in point 2. "Can I use this in some totally different place than I expect, even if I don't think I need to right now" is a good guideline to determine if you've written something clean.

The problem with #28 above is that it increases the static/global count. That makes it a non-starter, IMO.

Adding an active context flag internal to the Context class, as the #0 patch does, is what we did originally, and it was a problem because it broke unit testing. That behavior doesn't really belong in the Context class itself, because it makes it less reusable.

#1342192: Allow different class to be layered is a separate matter. We've already agreed that should get fixed as soon as a patch is posted for it.

So... can we improve point 3 without harming points 1 and 2?

It's more the ability of spawning a context tree outside of core context tree the real feature I'm talking about ever since the begining.

@chx I hear your concern, I really do, since the beginning. Moreover since you're one of the people that know most core internals, you're one of the most valuable insight, but this Context API may carry some complexity nevetheless it's encapsulated complexity that doesn't involve usage behaviors modifications upon the common use cases for most people.

The really hard part of this API right now is not these internal complex points (even there definitely is a lot of stuff to solve in there) but mostly the core glue that ties the contextes (handlers mostly) to the actual core code: this needs a lot of work. It's decoupled away from this internal complexity by key point interfaces (such as the HandlerInterface one) and can be worked out without having to worry about internal stack complexity.

Plus, the core handlers are the glue code which may imply changes on core, and these needs to be watched by non-WSCCI external eyes to ensure that they do not induce any major design breakage before the WSCCI got a chance to get further (REST and stuff, ...) or core didn't solve its own problems (session handling, etc..).

So as a layman can someone please answers the questions that have been asked - as it stands the replies to original patch seem to be based on fud rather than answering the question set out in real terms.

marcingy: #39 is the current status. If we can improve point 3 without harming points 1 and 2, I'm all for it.

That doesn't actually answer my question......

EDIT'ed the full post.

Context API standalone is only an arbitrary information container, that supports contextual per scope data layering and override.

There are tons of use cases for this, we could use this kind of API for local configuration override, component tree contextual information (components that inherits from their parents, code parsing, code interpretation, complex contextual menu creation), graphing, etc.. or many other business stuff.

My use case of such system recently was heavy migration scripts, component based, relatively comparable to the migrate module, except it was global state free and working with a deep component tree where each one of them could override the temporary storage layer, the default logger being used, the local configuration into its scope.

Asking for a detailed list of specific use cases of this seems foolish because 1) I don't know them, and 2) I cannot predict what weird stuff will come out of other's developers mind.

Hardcoding the core stack into it just remove all of this.

Right now, the Context API state is that all handlers are specific implementations that are injected at bootstrap, they dont taint the generic side of the API internals. If we hardcode the stack because 10 lines that seems complex (but believe me it's not) it'd be sad because it'd be the one and only one hardcoded core stuff into the full Context API code.

Just to be clear, this issue is about a detail that one and only one person spotted as complex, and raised a red flag that actually made a lot of people participate to this issue. But once again, to be clear, before anyone, I mean ANY one comes here and says that's complex, they definitely should read the rest of the code first, such as the traveral logic algorithm and the weird implementation of context inheritance (I dare someone that didn't followed the sandbox to fully understand that code at first read), both are like 1000 times more complex than this stack code.

My opinion is that removing the generic state of a full API because it suddendly became frightenning because this post has been made about it is sad. Don't misinterpret me, I'm not saying it's perfect or utterly simple: I'm OK to work on this and refactor, but I'm definitely NOT OK with chx's solutions because it taints the API hardcoding core specifics into it, and it looses the generic potential usage of it.

Let's start fight in issues about stuff that really are complex first. As long as we don't need other pieces of hardcoded core specifics into it, let's keep this one generic even if complex. I will definitely change my mind if the rest of the API starts to be filled in with hardcoded stuff, but today is not the case and most of the last monthes work worth was about keeping it generic and lightweight.

Please, do not make the last monthes debates, iterations, internal fights, hours of code of many people explode into pieces just because a tiny detail: harcode anything in there would definitely make the whole stuff vanished.

But without real use cases what infact are you modelling? What not adhere to the principle of keep it stupid and instead create something that meets the needs of the 80% rather than being all things to all people? And 'just' because one person has said it complex and no one else has does not make that persons point in valid. Maybe they are the only person who realised the real implications of what is being proposed.

And this comment

But once again, to be clear, before anyone, I mean ANY one comes here and says that's complex, they definitely should read the rest of the code first, such as the traveral logic algorithm and the weird implementation of context inheritance (I dare someone that didn't followed the sandbox to fully understand that code at first read), both are like 1000 times more complex than this stack code.

Fills me with dread - is this a good thing being 1000 times more complex and pretty much by your own admission unintelligible to pretty much anyone?

No, what I mean is that making the stack optional is the very last bit that decouples the Context API from core itself. In that matter, I would tend to say that we should keep this decoupled, it's a huge error to start to hardcode stuff while we still can keep it decoupled.

The stack handling is not complex at all, it's just that each parent context pass its own stack when spawning children so it makes us able to choose wether or not we want to inject a stack when creating the root context of the context tree. There is no more complexity than a setter and a getter.

If you don't understand that, I really don't know what to say. I guess yes, when reading the code the first time it might not be obvious, but once you just understood that each parent context injects its stack to its children so they can register to it via the Tracker isntance, it's all right!

I was proposing as a compromise to remove the stack, and make usage of it only in the tracker class: this would make things a lot simpler since the context wouldn't have to know about it (in fact it's chx proposal, but I extends it), but in order to keep it decoupled from core, we need the contextes to be able to use "non stack aware trackers" and the default behavior must be that.

Core is the only place where a developer will use that stack (because it's static, singleton, etc...), and bootstrap already occurs to set it up, so there is no need to make "stacked context tree" the default behavior since anyone on earth that would use its own context tree for its own usage can NOT use the core stack else it would mess up with core internals at runtime and create bugs.

That's why I propose that the Tracker class to be used by contextes should be configurable, and when a context creates a child, it pass its configuration to it: this would make everything being automatic and end developers wouldn't have to care about (at all) of this encapsulated and hidden mecanism.

This is really SIMPLE, don't say it's complex, it's not. Field API is complex, really complex, entities in D7 are (no full CRUD at the same place, every modules uses it differently, there is no existing documentation explaining the real design), form API is complex as soon as you start to mess up with it, Drupal ahah handling causes real headaches because when you start to do dynamic forms it all explodes.

Don't ever ever talk to me about complexity, the whole core is full of useless complexity. DBTNG is a nice piece of OOP code and a lot of people say it's complex, but really it's not: it has a complex business and it solves it in a nice way.

This piece of code of Context API we're actually talking about is a 10 lines piece of code, that is documented in a nice public interface, where everything is encapsulated, and based on a simple component inejction pretty much as a lot of frameworks do in 100 times more complicated since 1990.

Define complexity.

Know I see why chx closed the issue......

Sorry I'm being exhausted of this thread taking huge proportions, as I linked in a comment upper, there is a lot of really more complex stuff to look at. Handlers are one of them because they are the actual piece of glue that ties this Context API with core, and some of they may imply core design changes. These really are dangerous and need as many eyes as they can.

The real use case is core's currently scattered and disorganized state management, which is a nightmare. That needs to be cleaned up, which everyone agrees.

The primary target where this system will be useful is the part 4 "super block-based layout" system that is part of the WSCCI initiative; think panels in core but cleaned up and modernized. Much of the override and stacking logic is built with that in mind, and we've been talking with folks on the ctools team to make sure that it won't be a dead end in that regard.

Although that's the primary use case, it is architecturally foolish to hard-code it to just that use case. Portable code is a worthwhile goal, even if you don't immediately know where it will be portable to. It also makes it more unit testable, which in turn makes it more portable, etc. That's a very common best practice in software engineering, and one that Drupal should follow far more than it currently does. Loose coupling is a good thing. As I said in #39, use in other situations is not a direct goal, but it's a natural side-effect of a good, loosely coupled architecture.

Also, pounard is being somewhat hyperbolic. The traversal logic is not 1000x times more complex than the stacking logic. It is more complex, yes, but it's reasonably well-encapsulated, and should be documented inline. (If it's not, please file a documentation patch.) It's also less complex than many things already in Drupal. The frustration, I believe, is that these issues have been discussed at length over the past several months already, publicly and in the open, and revisiting them over and over again without specific bugs in hand to demonstrate really accomplishes nothing.

Sigh. Cross posting.

Yes you're right I'm hyperbolic, I'm trying to divert people's attention on what's really important and in that regard, traversal logic is because it's the central piece of the public API of contextes.

The Drupal way was to crate an API which covers the use cases we could think of (the proposed change does not change that) and leave an alter or similar (there is an accepted proposal for that too) for everything else.

Please, be serious.

Portable code doesn't mean we want to conquer the world winning a design contest, it means that contrib modules would be able to use it without having to alter it. Sounds like a good plan to me.

If they don't need to alter or extend it, then it's 100 times simpler, because the only thing they need to understand is the interface in order to use it. In the case they need to extend it, then they have to understand fully the internals to ensure they don't mess up with.

So I vote for a tiny bit more of complexity in the internals to ensure a clean interface for the rest of the world..

Portable code is a worthwhile goal, this doesn't mean we are all black or white. It just means that as long as the code *CAN* be indeed decoupled at a very low cost, we should keep it this way, because it solves my first point.

It's not about writing *only* portable code, it's about writing it *when we can do it at low cost*. In that particular case, it definitely worth it because it doesn't cost anything except the time we already took to think it.

It doesn't break any Drupal way, this is a false argument.

Ask for sanction whatever or whenever you want, but if you end up doing this each time someone dare impose himself in front of you, in 2 years we'll still be fighting for stupid details.

I know for sure now, after all those post and some clearly pejorative, almost insulting and personal comments towards me and Crell that you will continue to ask for sanction whenever you don't like something. So even if a sanction happens in here, you being debating this kind of issues is a never ending problem.

I really don't know what to say to you at this point.

pounard: Then please don't say anything further. You're really not helping this thread to be constructive.

Dries: Let me know if you want to chat about this thread and its background.

@Crell re: #32 I was thinking we'd used the class as a singleton rather than having a singleton instance. Maybe I've been looking at too much Ruby code.

I assumed we'd use a class variable, not a static variable.

However, I'm happy to see us use a singleton instance with a class factory method or something, but your comments seem to imply you have some notion of the desired architecture that we are all supposed to understand.

Look, I think D7 code is too complex and too hard to follow the code flow. I'm willing to sacrifice some flexibility or elegance if we can make it possible for a new contributor to read and follow the code flow.

@pwolanin Two important goals of writing object oriented code are separation of concern and encapsulation. Writing less code doesn't mean that it's more complex, because those two principles hides the complexity to the API user as long as he doesn't want to see it. This doesn't remove complexity but allows it to avoid spreading among the framework, and make it living into the minimum extent possible. Developping this way allows us to create complex but understanble because localized and encapsulated pieces of code at hard spots such as this one.

I think chx raises a good point in #62, that one of the things we've been traditionally lacking in the WSCCI initiative is some well-defined use cases. We are going to talk about this at the WSCCI sprint next week and define use cases. Also, depending on how we decide to tackle WSCCI, this architectural distinction may be irrelevant. In any event, we aim to provide a resolution for this by the time we're done with the sprint, and we'll report back with our recommendations shortly after. For now, I'll leave it assigned to myself so I don't forget to circle back on this issue.

Small note that in #1463656: Add a Drupal kernel; leverage HttpFoundation and HttpKernel, we're getting a Drupal Kernel class. We could make that the center-piece of all Drupal contexts. When #1497230: Use Dependency Injection to handle object definitions gets in, we'll be able to construct and lazy-load objects from a given context.

Yeah, I think this issue is officially dead. It pre-dates Symfony discussions and the other patches are going in a different direction.

It went wrong when someone used the word "singleton", and beside there was quite a different running between us at the time, I did say stuff I regret because there were not constructive, but I surely don't want to do this anymore. Let's put the past aside because Drupal 8 is so much different from what it was going to be at the time.

I'm quite opposed with putting named tags with your emotions inside, this is a community project not a Care Bears episode. Plus, this causes a potential scalability problem: Let's count:

• There's potentially 200,000 active user accounts on Drupal.org
• Everyone has 4 different emotions, and will use 1 to 2 on each issue
• There's 2,000,000 nodes on Drupal.org
• Every node is followed by an average of 5 to 10 people

That gives us:

• Always: 4 * 200,000 = 800,000 tags in term_data table
• Best case scenario 2,000,000 * 1 * 5 = 10,000,000 rows in term_node table
• Worst case scenario 2,000,000 * 2 * 10 = 40,000,000 rows in the term_node table

I guess drupal.org MySQL servers are robust enough for those tables; and considering that term_node is just an (int, int) table where its primary key is (int, int) it will be fast anyway, but that's still an awfull lot! Plus we are going to have a serious problem on taxonomy term autocomplete doing LIKE queries on the terms names.

Further updates to remaining tasks