Better usability for forced exports

[stephan]

Discussing the alternatives in #198 it appears that a more fundamental solution is desirable:

Files: Granting forced exports should happen through files in the workspace, not through the config.ini of the eclipse installation, specifically, some file(s) in ${workspace_loc}/.metadata/.plugins/org.objectteams.otequinox/

Confirmation: Should during launch some requested forced exports not be granted via the above file(s), permission may be asked from the user. For this purpose org.objectteams.otequinox should provide an extension point. An extension to this point will receive the forced exports request from the aspect plugin and should answer whether this request is granted. If neither file(s) nor extension confirm a forced exports request, an exception will prevent loading of the aspect bundle.

The above is the consensus between a proposal by ruwen and my current thinking, follow some open questions:

File(s)

File format:

• [ruwen]: use an xml file for the content and an .ini to refer to the .xml, example xml:

  <forcedExports>
     <plugin name="de.nordakademie.." version="0.1">
       <targetplugin name="org.eclipse..">
         <package>org.eclipse..</package>
         <package>org.eclipse..</package>
         <package>org.eclipse..</package>
       </targetplugin>
     </plugin>
     <plugin name="de.nordakademie.." version="0.2">
       <targetplugin name="org.eclipse..">
         <package>org.eclipse..</package>
         <package>org.eclipse..</package>
         <package>org.eclipse..</package>
       </targetplugin>
     </plugin>
  </forcedExports>
  

• [stephan]: use only one non-xml file, syntax in the vein of MANIFEST.MF, e.g.:

  org.eclipse.jdt.core
  [
    org.eclipse.jdt.internal.compiler.ast;x-friends:="de.nordakademie.aspectPlugin1",
    org.eclipse.jdt.internal.compiler.lookup;x-friends:="de.nordakademie.aspectPlugin1,fr.ordinateurs.octetsPlugin",
  ]
  de.nordakademie.aspectPlugin1
  [
    de.nordakademie.associations;x-friends:="org.objectteams.SuperPlug"
  ]
  

  Actually, this is the current syntax (config.ini) plus allowing line breaks.

• Implications:
  • File format can be optimized for tool-processing (XML) or readability by seasoned eclipse developers (plain text).
  • Content can be ordered by aspect plugin (a) or by base plugin and package (b). (a) is closer to the forced export request, (b) is closer to what is affected.

• Fine print:
  • Yes, it should always be possible to specify versions of all involved bundles.

Indirection: The actual file can be contained in the preferences directory or a .ini file can contain a link to a file referenced either locally or via arbitrary URL.

I prefer to have this information "manifest" in the workspace. Should fetching the content from some URL be desired, this could be the job of an extension while being asked for confirmation.

Through this file the user confirms the given requests. Pointing to a third-party location would say, I trust whatever they say. This would require some cryptographic protocal, which is not likely to be the job of the otequinox core. By contrast, a neutral extender plugin could inform the user:

"The request from plugin A conforms to what is posted on the trusted web-site AA (certificates have been checked). Do you accept?"

Confirmation

Howto handle multiple extensions to the new extension point?

One way would be to say that at most one extension is allowed per this point.

Otherwise, combination of several answers must be defined. I propose the common three-value logic for permissions: DENY, ALLOW, UNKNOWN:

• a single DENY suffices to prohibit loading of the requesting aspect plugin.
• a single ALLOW and no DENY suffices to allow loading of the requesting plugin.
• only UNKNOWN prohibits loading of the requesting aspect plugin (i.e., default is DENY).

config.ini

I currently think that the old way (config.ini) per eclipse installation plus its slight improvement from #198 should be kept so that granting forced exports can happen system wide (config.ini) or per user (workspace preferences).

Action upon denial

Not accepting a forced export could be "punished" at two levels:

• don't activate teams from the requesting plugin
• completely refuse to load the requesting plugin.

(Note, that a forced export can be utilized even without any team being activated by the framework).

Further wishes

• Yes, managing files (locations and content) through a preference page is desirable.
• The same extension point can in the future also ask for permission for regular aspect bindings w/o forced exports. This should then be configurable "Always allow", "always deny, "ask".

[stephan]

On a side issue:

r19600 stream-lined logging of OT/Equinox events, as to make log-level INFO the default, which now only reports user-relevant events.

[ruwen]

I forgot a little thing: We need a little hook when a plugin gets removed. In that case we also should remove the entry from our forcedexports.xml/ini.

@file format: When xml is used, a DTD should also be provided. And editing the forcedexports.xml using a good editor (which checks the file against the dtd) can avoid any trouble with the syntax. If we add the version to the current ini-syntax, it gets hard to read at some point. XML would be easier to read in my opinion.

@Indirection: I agree we do not need the possibility to access remote urls, since a little plugin can do that. We could also provide a little sample plugin which grabs the file without any encryption from a http-server. If people need more security, they can modify that sample plugin.

@Confirmation: since we do not have to refer to a remote xml anymore, we also could put the policy into the forcedexports.xml/ini, like:

<forcedExports>
   <settings>
      <policy>NO_DENY</policy>
   </settings>
   <plugin name="de.nordakademie.." version="0.1">
 ...
   </plugin>
</forcedExports>

Again, the policy could be checked using the DTD ;)

@Action upon denial: I would complety refuse to load the requesting plugin. It makes it easier to spot the error. If the plugin contains some "normal" code and some Teams, only half of the plugin would work properly, which maybe leads to strange exceptions or serious damage.

[stephan]

r19602 implements some first steps:

• support per-workspace configuration of forced exports (still using non-xml file format)
• initial support for DENY | GRANT | UNKNOWN DONT_CARE:
  • added new properties otequinox.forced.exports.denied and otequinox.forced.exports.granted (config.ini or commandline)
  • support distinct files deniedForcedExports.txt and grantedForcedExports.txt (workspace)
• OTStorageHook now reads and merges denials/granting from these locations:
  • config.ini (or system property set via command line, this is the same)
  • files specified from config.ini (or from command line)
  • files in the workspace's .metadata (s.above) -- this is triggered by the TransformerPlugin which -- as a Plugin -- can access the workspace.
• TransformerHook refuses to load classes from an aspect bundle with denied requests -> throws ClassNotFoundException with a hint to the denial. Specific errors regarding forced exports will have been logged before.

r19603 adds an extension point org.objectteams.otequinox.aspectBindingNegotiators, such that all registered extenders will be notified about any unconfirmed forced export (future: also aspect binding).

Each negotiator's answer contains three pieces of information:

• One of DENY, GRANT or DONT_CARE, where a request is granted only, if at least one GRANT is received and no DENY.
• A flag saying whether the answer (if not overridden by others) should be made persistent, so that the next bundle start will not require negotiation. Currently, the workspace files mentioned above are used for persistence.
• A flag saying whether the answer should be applied also to all like questions (i.e., either all forced exports or (future) all aspectBindings). This flag is not yet evaluated.

To this point, I still use the non-xml file format. Some considerations:

• Currently, storing negotiation results is done naivly by appending one block for each request to one of the files deniedForcedExports.txt / grantedForcedExports.txt. No consistency checks are performed, previous decisions are not deleted, nor are similar entries merged (e.g., regarding the same base bundle.
• From this point I don't really expect manual editing of these files as a normal use-case -- it should be possible but is not the main concern.
• If editing should be supported in the IDE we might, e.g., check whether the dependencies tab from the PDE plugin manifest editor can be reused for this task.

[stephan]

The relevant improvement in r19604 is (aside from code cleanup):

• Introduction of a per-workspace file negotiationDefaults.txt, which stores the default behavior regarding aspectBindings and forcedExports in two properties.

During the first launch, when this file does not yet exist, a new one is created with the following content:

aspect.binding.default=GRANT
forced.export.default=DONT_CARE

The effect is that all aspectBindings are allowed unless explicitly prohibited, all forcedExports are subject to approval by some other source.

However, checking of permissions for aspectBindings is not yet implemented.

[stephan]

Build 1.2.6.200902221503 is on the server (http://www.objectteams.org/distrib/otdt-updates-unstable) and contains all changes up-to and including r19603.

r19604 is still awaiting test results.

[stephan]

AspektAsker? plugin

[stephan]

The little AspectAsker plugin hooks into the new extension point org.objectteams.otequinox.aspectBindingNegotiators, informs the user about requested forcedExports and lets him/her choose from these options

• Don't care: aspect will be rejected unless another negotiator grants permission
• Grant: grant this one request.
• Grant all forced exports: grant all requests for forced exports.
• Deny: deny this one request.
• Deny all forced exports: deny all requests for forced exports.

Note that the OT/Equinox implementation currently ignores the "... all forced exports" supplement.

Additionally a checkbox "Remember my selection for subsequent launches" is provided. Selecting this checkbox causes the decision to be stored in the workspace.

The implementation of the message dialog avoids using JFace because that would interfere with system bootstrapping:

• Negotiators are (currently) invoked from the bundle activator of org.objectteams.otequinox.
• Aspect weaving only starts after the mentioned bundle activator has finished.
• If a negotiator depends on other bundles, these bundles will become unweavable.
• (Also any negotiator is unweavable).

The AspectAsker only depends on org.eclipse.swt, which should be tolerable.

Summary so far

The current implementation implements all requirements marked as consensus in the original description.

For reasons of laziness the file format has not been changed. A decision in this matter is still pending. Also bundle versions have not yet been added to the file format. I'm not even convinced any longer, that a user really wants to approve each new version of an aspect bundle. If a specific version is unwanted that version should simply not be installed, rather than installing and denying the necessary weaving.

An indirection is only allowed when this information is configured via config.ini (system property), where the @filename syntax has been introduced in #198. For workspace configuration we agree not to support indirections.

The negotation has been implemented using a three-value logic of DENY, GRANT and DONT_CARE where priority is decreasing from DENY to DONT_CARE. Thus combination of several answers is well defined and behaves as expected.

The old config.ini entry is interpreted as one negotiator (among others), i.e, information found there is combined with the information found in the workspace and received from extenders of aspectBindingNegotiator. The old mechanism has been enhanced to support denials, too.

It has been decided that a denied request will cause loading of the requesting bundle to fail, which is implemented by ClassNotFoundExceptions thrown for all classes from this bundle.

Remaining todos:

• decide about the file format.
• provide preference pages or alike to configure: (a) workspace defaults, (b) workspace configuration for specific forced exports. (Check the dependencies editor)
• apply the same protocol to aspectBindings, too. The proper default (GRANT) is already established in the workspace preferences.
• implement merging of forcedExports information in files and also removal of obsolete information.

One proposal I did not fully understand: what is the intended meaning of this (from comment:2):

<settings>
   <policy>NO_DENY</policy>
</settings>

Is that comparable to the file negotiationDefaults.txt in the current implementation? (see comment:4)

Bottom line

Given the AspectAsker plugin it is possible to grant all forced exports without ever manually editing a single file. It is trivial to implement another negotiator which always answer GRANT. Alternatively, changing one entry in negotiationDefaults.txt from DONT_CARE to GRANT can also enable all forcedExports in this workspace.

Thus I consider the original request resolved. Further enhancements can be tracked in separate issues, should the need exist. If, OTOH, the current implementation exhibits bugs or is otherwise incomplete please feel free to re-open.

[stephan]

Updates:

• The protocol has been extended to apply to plain aspectBindings, too: r19610.
• The constant DONT_CARE has been renamed once more to be UNDEFINED now (r19665).

[stephan]

As pointed out by ruwen (ticket:198#comment:4 - towards the bottom), versions might become relevant if different versions of an aspect plugin request different forced exports.

For convenience sake I'd for the moment stick to not differenciating versions, because I wouldn't want to ask for permissions after each update of an aspect plugin. So, currently permissions are treated accumulatively and apply to all versions of a given aspect plugin.