Completely hide MidCOM core level symlink support by default

Someone else (tm) besides the implementer needs to do some clicking around in the UIs to verify that the feature is indeed completely hidden to users unless explicitly enabled.

Said someone else (tm) will post their verification to this ticket and then it can be closed, that someone is personally responsible for any trouble I get in case their testing was insufficient.

Addennum: In case the best code required for hiding the property from Asgard reflected DM2 schemas is "just way too ugly" we can waive the invisibility for that part.

If you mean the feature must be hidden by default in the UI, this is simple to solve. Because basically it just means it must not show (by default) in the toolbar (one place) and the privilege must not show (by default) in Asgard (second place).

Did I get it right?

Got an answer from bergie in dev list discussion:

If you mean no clicking should show the feature

Yep, I think that is the idea. A bit similarly like approvals only

show up if they're enabled.

So this should be pretty easy I think.

(In [24562]) Hide MidCOM core level symlink support by default. Fixes #1512, refs #1548

To someone else (tm): Feel free to verify and close this now. :)

Remember to verify that the field appears to Asgard topic edit when feature enabled and disappaers when disabled. I do not see any handling related to this in asgard/handler/object/manage.php

(In [24576]) Hide topic symlink field in Asgard. Fixes #1512, refs #1548

Sorry, I don't know why I didn't notice that the topic symlink field appeared in Asgard. I thought I looked and didn't see it there, but there it was anyway of course...

The field actually needs to be always hidden in generic tools like Asgard because only midcom.admin.folder knows how to touch it correctly and it makes no sense adding its handling code to every tool out there.

It is now hidden in Asgard. If there's some generic way to define its schema (or something) in a way the field will automatically be shown only in midcom.admin.folder and no place else, that would of course be a better solution. But I'm unaware of such solution existing so I just hide it in Asgard.

Anyway. It's now hidden.

To someone else (tm): Feel free to verify and close this now. :)

Configuration switch works nicely via midcom.conf file. Symlink topics are created correctly with good sanity validation.

I was a bit unclear on what everything means. Basically all code blocks handling symlinks should be behind a check if the feature is enabled (case point #1583).

To put it other way around, no symlink handling code may be executed if the feature is disabled.

At the *very* least everytime there is something along lines of:

if ($topic->symlink)

It must also check if the feature is enabled and thus changed to:

if (   isset($GLOBALS['midcom_config']['symlinks'])
    && $GLOBALS['midcom_config']['symlinks'] === true
    && $topic->symlink)

This is to make sure no magic happens even if symlinks were previously enabled but are now disabled.

This needs to be properly thought out. I will disable as much as is possible. There *might* be some checks which just can't be disabled because they keep the site together so to speak. But most of the code can be disabled without that kind of worrying.

Then there's also the question of whether to e.g. tell user that "this is a symlink" when deleting folder if symlink feature is disabled. Should it just be silent or tell it. I'm not sure. It might make sense to tell about it even if it is not followed because disabling the feature will not reset the symlink property values of course. But that's a bit a matter of taste also what to do there.

It might make more sense to not tell about it because in theory the folder could be reused for something else (possible from Asgard)...

r24676 and r24690 should have referred here

You really didn't say that originally. I even asked what you meant and as you have commented after my question I take that as an acknowledgment that my understanding was correct.

So this ticket was originally about hiding the feature from the end user when it's disabled. And that to make it disabled by default.

Of course also disabling magics related to the feature was part of it (when it's directly related to end user). That was implemented by disabling usage of the feature when feature is disabled. The code doesn't do anything if you don't have symlinks (and basically there's no code executed as the code always begins with checking if topic is a symlink). And if you have symlinks then you have them. Disabling the feature doesn't remove the created symlinks anyway. Not following them doesn't really help the end user as such folders need to be manually deleted anyway then. (There doesn't seem to be a real life use case.)

#1583 doesn't prove anything else than that I made a simple mistake and forgot somehow to change one "guid" to "id". Unfortunately it caused an ugly bug - which is fixed now and a hotfix is released as well - but that is besides the point.

Now you changed this ticket to be about disabling the internal functionality as well which is totally different thing than hiding from UI. Now you basically want to have the same functionality as Apache HTTP Server has with its FollowSymlinks.

That is doable but it's a new feature. #1512 was about adding same kind of functionality as symlinks in the filesystem. You can't disable symlinks from the filesystem (at least that is not done in practice). We can disable them from MidCOM and I will probably do it. But it's a new angle to this.

Also, the feature itself is designed to be fully "hidden" to the user if it's not used. FollowSymlinks kind of feature is not required to safeguard against this feature. The existing code is already safe if feature isn't used. Adding FollowSymlinks kind of feature does not help in any way to prevent bugs or similar. So if that was the use case, it's invalid.

The "hide" currently means "do not allow creating and editing symlinks". Now you want it to mean the same as "do not allow creating and editing of symlinks *and* do not follow symlinks". You want me to code a new feature which is the same as mounting a filesystem with symlinks disabled (is that even possible, I don't know and it's not relevant to this of course) or like what httpd has with its FollowSymlinks. That is perhaps a good idea but it really has nothing to do with hiding the feature from users.

Implementing the feature will not improve stability in any way. It's just a new feature. It might even introduce new bugs (if also internal checks are disabled when feature is disabled - especially if user enables the feature again after it being disabled in between).

The original use case (and the point of this ticket from my point of view at least) was to protect existing installations against this feature (not to affect them or cause wondering about what the feature is etc.). Now this is beginning to look more like a feature request which is related to actually using the feature itself (you see, totally different angle). I wonder if there are really any real life use cases for the feature you are requesting... It might be kind of nice in paper but in practice... Not really. It's just more work without actual gain (as I said, implementing this doesn't improve stability or protect users any better than they already are).

(In [24695]) Revert r24676, at least for now, because it's not consistent with the current behaviour. Symlinks are currently followed always if they exist. That means that their name syncing must also be in use (otherwise it's very strange for the end user who edits the symlink from the UI). This can be set back if symlink following is disabled when symlinks config option is set to false. That is not the case currently. It is still a bit unclear if such feature is needed but if it is, then this can be set back together with the following code. Currently, having it this way is the absolutely right thing to do. Refs #1512, refs #1583, refs #1548

(In [24715]) Enable the createlink plugin handler only when symlinks setting is enabled. Refs #1512, refs #1548

Replying to jval:

It might make more sense to not tell about it because in theory the folder could be reused for something else (possible from Asgard)...

r24693 handles the use case of reusing the folder.

Well partly. I will do the same for midcom.admin.folder was well.

(In [24728]) If a symlink topic is reused as a real topic, reset the symlink property as then it's not a symlink topic anymore. This prevents magical symlink following when user doesn't expect it. Refs #1512, refs #1548, refs #1583

(In [24729]) Add an empty option to the beginning of the component selection if current topic doesn't have a component (symlinks don't). Refs #1512, refs #1548

(In [24730]) Use URL name for the title if extra is missing (symlinks don't have extra set). Refs #1512, refs #1548

I've thought about this a lot and here's my conclusion:

1. The most important thing is stability, and from the actual end user perspective. MidCOM should not break the site when administrator uses the software/framework in a normal way. Enabling and disabling a configuration setting is normal usage. MidCOM should work properly in both states of the configuration setting. Certainly, site breakage should not happen. A software which functions in a way it breaks in a normal usage is just simply implemented wrong. That kind of behaviour would just simply be a bug - *not* a feature. :) So, this should be implemented in a way normal usage doesn't break the site. Enabling and disabling a feature must not cause any failures.
2. This feature is not a module. It's a patchset to MidCOM Core and to the administration user interfaces. The feature can't be just *fully* disabled when the configuration setting is disabled. This thing must not be thought like #ifdef with C programs (where one of its use cases is to make it possible to enable or disable features fully during build). Instead, we must think this from the end user perspective. Certainly, disabling should disable something and in an expected way. But it must not behave in a way the site can break while testing the feature and playing with it. (I bet it would be quite normal usage to enable the feature, do some tests, disable it and later enable it for production. That kind of usage must not cause the site to go down.)
3. To protect the site from breakage MidCOM needs to check that symlinks, when they exist, are used in a healthy way. Cross linking in a way an endless loop forms must not be allowed and must not happen in any circumstances because the site would break fully otherwise.
4. Disabling the symlink support doesn't remove existing symlinks (if feature was previously enabled and symlinks were created). They are symlinks regardless of the state of the setting. They need to be kept in a consistent state (checks must be executed if symlinks are encountered). Of course also reusing the topics should be taken into account. If user has changed a topic to a normal one it must stay as one and must be handled as one. But because symlinks can't be ignored they must be handled as long as they exist and are indeed symlinks. See http://trac.midgard-project.org/ticket/1583#comment:15 and http://trac.midgard-project.org/ticket/1583#comment:16 about the "handling" angle. That was a bit on the grey area but it was still on the side where it makes more sense to do it always. The checks are fully clear: they *must* be always executed to keep everything together. It just simply isn't an option to disable those as we *must* protect the site and keep everything working.
5. So... This all comes down to the question: What is the expected behaviour? Well... As I told earlier, disabling symlink following isn't related to hiding the feature from existing installations. But, I've thought about that feature and it seems more logical to disable following as well when the feature is disabled. That's because the end user expects that disabling a feature completely disables it. Naturally symlink following is part of the feature. Also, I think it's more useful to handle it that way because if you want to control symlink creation permissions and similar, the privilege is for that - this disabling thing is more related to not using the feature any more. Then it actually makes more sense to make it possible to easily change symlink folders to normal ones (which requires that symlinks aren't no longer followed). Disabling the checks and internal integrity features on the other hand is not expected as that would break the site or as with the URL name syncing case, stop symlinks being handled properly (remember: they are symlinks after all and that touches only symlinks - you can change them or remove them but as long as they are symlinks they are symlinks - also remember that when they aren't followed the switch does inactivate them and so behaves just like expected). The actual feature should be disabled fully from the end user perspective and that means in practise that symlink administration features should be hidden and symlinks themselves should be inactivated when the setting is disabled.
6. The behaviour of the setting itself is not related to site stability. It's related to the question about expected behaviour. I have self-audited the symlink related code now fully, fixed and improved it. I can say for sure that the feature doesn't make MidCOM less stable. This work has actually made certain things better also generally (I've noticed other issues along the way and the code has been improved/simplified where necessary - mostly though, the feature didn't really change anything at all - it's almost entirely an addition - which doesn't even execute anything if you don't have the setting enabled and/or have symlinks in your system).
7. When the feature is disabled, there's the question about what to do when symlinks are encountered in the UI. Because symlinks can't fully be ignored as they are symlinks even if they aren't used/enabled/active: It is best to inform the user that a folder is a symlink when it is one. And to not cause confusion there should also be a message saying "symlinks are currently disabled" in such cases - wherever it makes sense. (It depends where you are in the UI. Folder edit should not tell about it because that's exactly the place where you can change the folder from symlink to a regular folder. It shouldn't look like you're editing a symlink as it's not a symlink anymore if you set a component there. Well perhaps it could tell it in some way as it's a symlink until it's changed to something else but I've looked at the code and it seems it's much simpler just to make sure the component is shown as empty in those cases.)

So, I'm going to make the feature fully disabled from end user point of view when setting is disabled but I'm not going to disable the internal checks and handlings because it's not what the end user would expect and it also just can't be done as the feature is part of core and it must be stable.

This means that symlink following is going to be disabled also when the setting is disabled and that the UI informs about symlinks and the state of the setting. Folder reusage support is already implemented in the previous commits so the feature behaves consistently (like expected) in both states.

After that is done, the feature is completely hidden from the end user perspective and the switch functions in an expected way. The feature itself is stable and it is safe to use MidCOM with this code in (whether you use the feature itself or not).

The feature was hidden from UI in 8.09.7(.1) but the rest of the work is done in the next release, so I'm correcting the milestone now.

(In [24738]) Fully inactivate symlinks (do not follow them) when the symlinks setting is disabled. Inform about the disabled state in the UI when a symlink is encountered. Fixes #1512, fixes #1548

(In [24739]) Use URL name for the title if extra is missing (symlinks don't have extra set). Fixes #1512, refs #1548