Bug 370903 - need richer set of validationProperties and URL transforms for various command extensions
Summary: need richer set of validationProperties and URL transforms for various comman...
Status: RESOLVED FIXED
Alias: None
Product: Orion (Archived)
Classification: ECD
Component: Client (show other bugs)
Version: 0.4   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 0.5 M1   Edit
Assignee: Susan McCourt CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 365649 (view as bug list)
Depends on:
Blocks: 341540
  Show dependency tree
 
Reported: 2012-02-08 00:37 EST by Susan McCourt CLA
Modified: 2012-04-20 16:05 EDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Susan McCourt CLA 2012-02-08 00:37:26 EST
right now we use validationProperties in orion.edit.command and orion.navigate.command to describe when a command is valid.  Then we map those properties to a visibleWhen function when we convert the service extension to a command.

We need to add some simple parsing to this support for commands that are simply composing an href from an item.  The issue is that we need to describe the transform rather than call the plugin implementation to do the parsing.

As an example, here are some "URL translations" that I had to supply as a non-extension command so that I could supply the visibleWhen block with code.  If there is a way to express this in validationProperties I didn't know how.

visibleWhen : function(item) {
	return item.GitUrl && item.GitUrl.indexOf("https://github.com") === 0;
}
visibleWhen : function(item) {
	return item.GitUrl && item.GitUrl.indexOf("git.eclipse.org/gitroot") > -1;
}

Once we could do this, I could move commands like orion.git.gotoEclipseGit and orion.git.gotoGithub into gitPlugin.html rather than writing js code in gitCommands.js

And if you look at the hrefCallback implementation, you can imagine that the whole transform from item.Location to href could be done with something like a URL template.
Comment 1 Susan McCourt CLA 2012-02-08 01:14:47 EST
the validationProperties would be the Orion equivalent of eclipse.core.expressions.  We should probably look at it for ideas.
Comment 2 Susan McCourt CLA 2012-02-08 16:44:11 EST
I would like to be able to validate with an OR such as

item.Git.CloneLocation OR item.CloneLocation

I had to make two separate command contributions in able to go from file metadata to the clone page, and from repo metadata to the clone page.  Maybe that's to be expected, since the logic for creating the href would also be differnet.
Comment 3 Susan McCourt CLA 2012-02-09 11:13:42 EST
another use case for "expressions" + "parsing" besides href construction:

in bug 366234, we may provide contextual tooltips.
If we wanted to let the command extensions participate in this, we'd have to have a way to describe how to get from "item" to "tooltip".  This seems similar to me in spirit.

Describe aspects of the item that are important and produce a contextual string, be it a tooltip or an href.
Comment 4 Susan McCourt CLA 2012-03-02 16:50:27 EST
I'll drive this one but will need Simon's help to see how URI templates fit into all this.
Comment 5 Susan McCourt CLA 2012-03-06 23:16:27 EST
In bug 372914 I invented some new mechanisms/transforms that can go from item URL.  In that extension, we provide a template that expands on the metadata plus some other known variables to describe how to invoke remote content.

We also have a case where we pass a URL to a plugin that will be called when the plugin saves its data.  It'd be nice for the plugin to provide some kind of regex or template to explain how to get the data out.

For example, if I know pixlr is going to call a URL that contains either:

"&image=http://foo.com/myImage" or
"?image=http://foo.com/myImage"

how can my pixlr plugin say...

"look for either &image or ?image and then grab the value after = and if there is another '&' afterward, stop.

Note we can't use our URITemplate parsing because the pixlr is completely unaware of our conventions.  What i did was hacky but works:
		

- tell pixlr to use "someOrionURL#resource,otherClientParms,imgapi" as its return URL (so that I know its image ref gets put in our client parms)
- the plugin specifies a tokens that should be used to find the return URL
	saveToken: ["imgapi?image=","imgapi&image="]
- the plugin specifies a terminator.  If either save token is found, where do we stop?
	saveTokenTerminator: "&"

So the whole thing looks like this:
provider.registerServiceProvider("orion.page.content", {}, {
	id: "orion.pixlr.content",
	name: "Pixlr",
	saveToken: ["imgapi?image=","imgapi&image="],
	saveTokenTerminator: "&",
	uriTemplate: "http://pixlr.com/editor/?image={OrionHome}{Location}&referrer=Orion&title={Location}&lockTitle&target={SaveURL}imgapi"

What's weird is you start to get multiple api's parsing URL's.  The pixlr URL used for save will use ?image if it doesn't see a '?' and will use &image if sees a '?'.  So it's making its own decisions about how to append its parameter.  So I set up the plugin to provide a saveurl with a special token, and the plugin uses its knowledge of pixlr's api to specify saveToken and saveTokenTerminator.

I guess the new thing here is that it's not just crafting a URL using a template and some regex type matching on the metadata, but also specify a way that someone else could read a URL and arrive at the same conclusion.  Like...use a template to craft a URL and supply a template that could be used to parse the returned URL
Comment 6 Susan McCourt CLA 2012-03-09 16:26:17 EST
I spent a little time looking at the cases.
I'm starting to think that we have most of the pieces and just need a better separation of concerns.

For example, consider this hrefCallback that makes a github repo from some metadata

hrefCallback : function(data) {
  var url = /github\.com.*\.git/.exec(data.items.GitUrl)[0];
  //convert : to / if needed
  url = url.replace(':', '/');
  return "https://" + url.substring(0, url.length-4);
}

the things at play are
1 - the json property "GitURL"
2 - the validation regex
3 - some string substitutions
4 - building a final result

we know there are cases where we want 2,3,4 to apply to different json properties in different circumstances (CloneLocation OR Git.CloneLocation). 

And we might end up with cases where we care about the validation for some other reason (is this a github repository) without wanting to do any URL construction.  So maybe we should talk about "validations" that can be used by "linkGenerators" and applied to "properties."

/* my git hub repo validation */
{
id: orion.GitHubLink,
validation: /github\.com.*\.git
}

/* my git hub link generator */
{
id: orion.GitHubLinkMaker
validation: orion.GitHubLink,
substitutionPatterns: [[/:/, "/"], [/.git/,""]],
href: "https//%1"
}

/* finally, a particular extension point ties the json to the validation and parsing */
{
validationProperties: ["GitURL": "orion.GitHubLink", "Name": "foo"....list all properties you want to validate]
href: orion.GitHubLinkMaker
}

Seems like we could unify link scanner with this, since really the "words" part of link scanner is just a way to identify the parameters.

John/Mark...thoughts?  You guys are way more regex savvy than me, there might be away to reduce the steps I described.
Comment 7 Susan McCourt CLA 2012-03-12 14:05:35 EDT
Separating out the properties to be validated vs. the validation logic seems like it will be very important when we want some extension to be able to reference a validator supplied by someone else.  But I'm not sure we have this case yet, so in the interest of backward compatibility, I'm going to prototype some extensions to the existing validation properties that do the following:

- you can pipe the validation property name to achieve the OR
"CloneLocation|Git:CloneLocation"

- you can use a real regex in the validation properties rather than file wildcard oriented one.  Strings will use the old file wildcard pattern, literal regex will be used as is.  So you could say::

{"GitURL|CloneLocation|Git:CloneLocation": /github\.com.*\.git}

- we could still support href but we could also have hrefProperties....I'm not sure we have a case where an href is built from more than one property name, but we should be prepared for it.  Here's an imaginary one.  Note that hrefProperties could be used instead of validationProperties...we could validate with it and then use it again for the substitution (and we could be smart about saving the results so that we don't validate twice).

hrefProperties: [
   "GitURL|CloneLocation|Git:CloneLocation": /github\.com.*\.git/, 
   "Name": /+/
],
replacements: [
   [{pattern: /:/, replace: "/"}, {pattern: /.git$/, replace: ""}], 
   [/* no substitutions for name*/]
],
href: "https://%1?user=%2"

I keep thinking there must be a better way to do replacements...
Comment 8 Mark Macdonald CLA 2012-03-14 03:40:14 EDT
(In reply to comment #7)
I don't have much to add, but posting a followup to our IM conversation.

I was just getting comfortable with the URI Template syntax, it seems a shame not to reuse it for constructing the href. If we made the substitution result into a template param, then the 'link generator' extension would become
  uriTemplate: 'https://{1}'
rather than
  href: 'https://%1'
A minor difference but at least it's one less replacement syntax to remember.

I found the numbering of substituted params hard to wrap my head around. You have to walk back several steps to work out which JSON param(s) they're ultimately drawn from. But I can't see a nice way around this. You could have the link generator give its params meaningful names, but then the calling extension will have to bind its validation properties to those param slots. The example would end up looking like:

/* my git hub repo validation */
{ id: orion.GitHubLink,
  validation: /github\.com.*\.git/
}

/* my git hub link generator */
{ id: orion.GitHubLinkMaker
  validation: orion.GitHubLink,
  substitutionPatterns: {
    'GitRepoURL': [[/:/, "/"], [/.git/,""]],
    'GitHubUser': [] /* no substitutions */
  },
  uriTemplate: 'https://{GitRepoURL}?user={GitHubUser}'
}

/* finally, a particular extension point ties the json to the validation and parsing */
{ validationProperties: {
    "GitURL|CloneLocation|Git:CloneLocation": "orion.GitHubLink",
    "Name": "foo", ...
  },
  href: orion.GitHubLinkMaker
  hrefBinding: {
    'GitRepoURL': "GitURL|CloneLocation|Git:CloneLocation",
    'GitHubUser': "Name"
  }
}

I'm not sure the added verbosity is worth it.
Comment 9 Susan McCourt CLA 2012-03-26 09:04:41 EDT
Another question here is regarding contentTypes.
orion.navigate.openWith can specify an array of content types that validate the item.  Do we want to unify contentTypes and validationProperties so that all extension expressions can use contentTypes as a possible validation property?
Comment 10 Mark Macdonald CLA 2012-03-26 09:55:38 EDT
(In reply to comment #9)
> Another question here is regarding contentTypes.
> orion.navigate.openWith can specify an array of content types that validate the
> item.  Do we want to unify contentTypes and validationProperties so that all
> extension expressions can use contentTypes as a possible validation property?

Does this mean extensions that currently use contentTypes would switch over to validationProperties (with 'contentTypes' as a property)? Or would contentTypes just be one additional property for extensions that already specify valdation properties (commands, etc)?
Comment 11 Susan McCourt CLA 2012-03-26 10:09:50 EDT
(In reply to comment #10)
> (In reply to comment #9)
> > Another question here is regarding contentTypes.
> > orion.navigate.openWith can specify an array of content types that validate the
> > item.  Do we want to unify contentTypes and validationProperties so that all
> > extension expressions can use contentTypes as a possible validation property?
> 
> Does this mean extensions that currently use contentTypes would switch over to
> validationProperties (with 'contentTypes' as a property)? Or would contentTypes
> just be one additional property for extensions that already specify valdation
> properties (commands, etc)?

Hadn't thought about it yet, was just making the note to think about it.  Probably the latter, so that someone who just needs content types wouldn't have to absorb/understand validation properties.  Not sure, though.  What do you think?
Comment 12 Mark Macdonald CLA 2012-03-30 09:56:03 EDT
(In reply to comment #11)
> Hadn't thought about it yet, was just making the note to think about it. 
> Probably the latter, so that someone who just needs content types wouldn't have
> to absorb/understand validation properties.  Not sure, though.  What do you
> think?

Yes, I was thinking along the same lines.
Comment 13 Susan McCourt CLA 2012-04-03 13:13:02 EDT
So here's what I'm working on this week, based on comments from Mark.  The goals are:
(1) - extensions that currently supply an "href" should instead supply "uriTemplate".  This lets extensions like orion.page.link, orion.navigate.command, etc. take advantage of template variables like {OrionHome} etc.

(2) - extensions that validate on a json object should use the same API, and this should include both validation properties and content types.  We need to define the order in which these are considered (the fastest?)

(3) - extensions that build URI's from json objects should use the same API, and this involves regex plus replace params.  As Mark suggests, we should stick with uriTemplate syntax for parameter substitution.  Logical names are better than numeric names when doing substitution.

This would definitely affect:
orion.navigate.command
orion.edit.command
orion.page.content
orion.navigate.openWith
orion.page.link
orion.page.link.related

We should look at orion.core.linkScanner after the fact and open a bug for what should happen with it.  Seems like it should use similar URI template syntax.

What we'll do:
- validation properties is an array of "validationProperty" objects.  These objects have the following properties:  
   * source (required) the name of the property in the item being validated
   * match (optional).  The value that should match.  If not specified, it simply means the source property must be present.  If it's a non-string value, we just use equality against the source property.  For string value matches, this is always a regular expression.  Mark pointed out we can't use literal regex in json, so we would have to escape it if we want to support both regex and non regex string values.  Since everything in the current string validation properties can be expressed as a regex, we can abandon the file-based wildcard syntax and just say that the validation value is always a regex if it's a string.  So we just take the string and pass it to the regex constructor.
   * variableName (optional) for future template expansions.  Extensions that aren't going to expand a uri from the validation wouldn't bother to specify it.
   * useMatch (optional)  A boolean that specifies whether the regex match or the non matching part of the original property value are fed into the variable.
   * replacements (optional)  array of substitution strings to apply to the match.

The complex case is where there is more than one variable.  This is contrived, but somewhat believable.  

validationProperties: [
  {source: "GitURL|CloneLocation|Git:CloneLocation", 
   match: "/github\.com.*\.git/", 
   variableName: "GitRepoURL", 
   useMatch: true,
   replacements: [[":", "/"], [".git",""]]},
  {source: "UserName", 
   match: ".+", 
   variableName: "GitHubUser",
   useMatch: true}
],
uriTemplate: "https://{GitRepoURL}?user={GitHubUser}"

The case where you want the non match part is covered in the gitrepo->eclipse link case.  Such as:

validationProperties: [
  {source: "GitURL|CloneLocation|Git:CloneLocation", 
   match: "git.eclipse.org/gitroot", 
   variableName: "GitRepoURL", 
   useMatch: false}
],
uriTemplate: "http://git.eclipse.org/c{GitRepoURL}"
Comment 14 Susan McCourt CLA 2012-04-11 13:55:19 EDT
pushed fix.
See http://wiki.eclipse.org/Orion/Plugin_API_Changes/R0.5 for documentation, there are a few new concepts not discussed in this bug that I added to support the github and eclipse.org href examples.
Comment 15 Mark Macdonald CLA 2012-04-20 16:05:07 EDT
*** Bug 365649 has been marked as a duplicate of this bug. ***