Description
Adding a branch in a scp-style url leads to a wrong ref property

To Reproduce

const url = new GitUrl('[email protected]:company/repository.git#products/v2');

url.ref is products/v2.git

Expected behavior
url.ref is products/v2

Subtask of #22

None of the origin properties are documented and the proxystrain schema lacks a good link to the origin schema documentation.

the .git suffix in the string URLs is a bit confusing. it stems from the fact, that git clone always uses a .git suffix:

https://www.git-scm.com/docs/git-clone#_git_urls_a_id_urls_a

and the url that github lets you copy-paste for the clone command, also has a .git suffix.
but the github API and UI never uses the .git suffix in the paths.

the helix-config.yaml requires .git suffix for string urls. in the object form, the repo property does not have a .git suffix. but toString() of the GitUrl always ensures the .git suffix.

potential changes:

1. make .git suffix optional in strings, but always add it for toString().
2. make .git suffix optional in strings, and always remove it for toString().
3. make .git suffix optional in strings, and keep the same for toString() (roundtrip)

The devDependency eslint-plugin-import was updated from 2.16.0 to 2.17.0.

🚨 View failing branch.

This version is covered by your current version range and after updating it in your project the build failed.

eslint-plugin-import is a devDependency of this project. It might not break your production code or affect downstream projects, but probably breaks your build or test tools, which may prevent deploying or publishing.

Your Greenkeeper Bot 🌴

@trieloff but then, those are wrong:

Originally posted by @tripodsan in #94 (comment)

Unify the expression of conditions so that:

1. no VCL code has to be written
2. we can reason to find out which condition is more specific (they should be evaluated first)
3. we can aggregate and combine conditions.

Basic Boolean logic:

• and
• or
• not

Property Matching:

• url
  • hostname
  • path
• referer
• client_name
• client_city
• client_country_code
• user_agent
• accept_language
• url_param.<name>

All property matching is using substring-starts, so only substring matches starting at index 0 are considered. More concise matching operations can be enabled through the suffixes: url= (strict match) and url~ (regex match)

Property Ranges:

• client_lat>, client_lat<, client_lon>, client_lon<

Suggestion

• Finalize the spec and write a grammar and JSON Schema
• Implement a transformer that turns a condition config object into a Condition object with an AST
• add methods¹ to AST Nodes that evaluate a request object. (for helix-simluator)
• add methods¹ to AST Nodes that produce VCL code
• expose condition via Strain.getCondition() and provide methods to evaluate objects and to produce VCL code.

¹ Alternatively, use a visitor pattern.

Notes

We would have a new Condition class that adds following functions:

• toFunction: creates a Javascript function that evaluates the condition against an express-style request object
• toVCL: generates a VCL snippet

We can then further subdivide it into PropertyCondition and BooleanCondition. PropertyCondition simply has two properties: property and condition and two private mapping tables of property to JavaScript function and property to VCL string.

BooleanCondition has a list of children and an internal mapping table for the prefix, suffix, and infix (e.g. ! is a prefix, and && is an infix)

Appendix A

Support to calculate base-url

For conditions that select a strain based on URL, it is important to determine the common root path of the URL that matched the request. this base-url acts as chroot for resource processing.

Examples:

(the regexp support might be an advanced feature...)

So it might be better to return a ConditionResult from toFunction()(req) that contains some method to determine the base url. eg:

const res = strain.getCondition().match(req);
if (res) {
  const baseUrl = res.getBaseUrl();
  ...
}

for VCL, the condition should be able to produce a script that calculates the baseURL.

but then the conditions schema is wrong, as it must only allow and, or, not as toplevel property.

Just had another look at the schema. conditions and not are objects, but and and or are arrays. The only problem is that conditions and not can have multiple properties, and they should be allowed to have only one.

Originally posted by @trieloff in #20 (comment)

Description
the schema mandates a ref in the code, content and static.url objects, but not in string alternatives. this should be consistently handled, and by adding a missing the ref to all strain giturls.

also see #82

The devDependency eslint-plugin-import was updated from 2.17.2 to 2.17.3.

🚨 View failing branch.

This version is covered by your current version range and after updating it in your project the build failed.

eslint-plugin-import is a devDependency of this project. It might not break your production code or affect downstream projects, but probably breaks your build or test tools, which may prevent deploying or publishing.

Your Greenkeeper Bot 🌴

Is your feature request related to a problem? Please describe.
I want to use this library to programmatically read the helix config, but I don't know where to start other than reading the source for the project.

Describe the solution you'd like
I would like a couple-liner node.js example in the README on how to use the library to read a helix config.

Strain.params is a whitelist of URL parameters. It works for Runtime and Proxy Strains. helix-shared does not have support for it right now.

Is your feature request related to a problem? Please describe.
for testing log output of the cli commands, it would we nice to have a string-only transport, so the test log files don't need to be read. this also would work around the flushing issue in winstonjs/winston#1504.

Describe the solution you'd like
when specifying /dev/text as logfile in Logger.getLogger(), a string based transport is created.
there should also be a way to retrieve the output, maybe by overriding the close() method so that:
const output = await log.close() returns the output.

Additional context

• see adobe/helix-cli@ce67700
• see adobe/helix-cli#433

add/move common logging functions from helix-cli and petridish

the current schema defines the condition values as properties:

which would lead to ugly trees for alternatives:

conditions:
  or:
    url.hostname=: www.adobe.com
    or:
       url.hostname=: www.adobe.io
       or:
         url.hostname=: www.adobe.net

of course, it could be written in a regexp, so maybe this is not really a problem

conditions:
   url.hostname~: www.adobe.(com|io|net)

an alternative could be to define the or, and nodes as arrays:

conditions:
  - or:
     - url.hostname=: www.adobe.com
     - url.hostname=: www.adobe.io
     - url.hostname=: www.adobe.net

or maybe both should be possible?

• add github token
• add npm token

see https://semantic-release.gitbook.io/semantic-release/support/faq#why-is-the-package-jsons-version-not-updated-in-my-repository

as discussed during the 4/2018 hackathon, the static directory will be htdocs. it will not be configurable via CLI or helix-config.yaml.

See adobe/helix-cli#587 (comment):

strains: 
  default: 
    redirects: 
      - 
        from: /old/
        to: /new/
      - 
        from: /old/(.*)
        to: /new/$1

• add a new array property redirects
• each array value has from and to properties
• from is a path or regular expression
• to is a regular expression replace pattern

The GitURL schemas defines a default value for the ref property. But the ref property is defined as required. This is somehow contradictory and misleading (if there is a default, this means you do not need to specify a value, the default will be used). The default value must be removed.

See:

• https://github.com/adobe/helix-shared/blob/master/src/schemas/giturl.schema.json#L53
• https://github.com/adobe/helix-shared/blob/master/src/schemas/staticgiturl.schema.json#L53

The HTML class attribute should be treated as a space-delimited set of strings, not as a string when comparing, so that <div class="foo bar"> and <div class="bar foo"> are considered equal.

[hlx] info: starting project
[hlx] info: Local Helix Dev server up and running: http://localhost:3000/
(node:99684) DeprecationWarning: The endpoint 'yaml/map' will be removed in a future release.
(node:99684) DeprecationWarning: The endpoint 'yaml/pair' will be removed in a future release.
(node:99684) DeprecationWarning: The endpoint 'yaml/seq' will be removed in a future release.

it would be good to get rid of those.

$ hlx --version
0.14.3

YAML forbids tabs: http://yaml.org/faq.html but our config files are read (with the wrong object nesting) when tabs are used instead of spaces.

see #31 (comment)

Description
toYAML() shows error: YAML_MAP is not a constructor

To Reproduce
tbd

Expected behavior
no error

/cc @stevengill

From: https://docs.fastly.com/guides/conditions/troubleshooting-conditions

Have you escaped forward slashes? Forward slashes don't need to be escaped in Varnish regex.

We should make sure to return non-escaped forward slashes.

Unfortunately comparing dom trees while discarding insignificant whitespace changes is rather more complex than anticipated and there is no library that decently implements whitespace collapsing
for html trees.

Description
have a simple strain:

default:
  repo: test
  owner: tripodsan

and run hlx up. you see that it tries to load content from localhost.

Expected behavior
as soon as a repo or owner is set, it should default to https://github.com

Version:
0.9

Additional context
workaround: the the default content. eg content: https://github.com/tripodsan/test.git

With #20 (conditions language) we might have a good shot at option 4 (which I think is preferred by @kptdobe): sort the strains from most specific condition to least specific condition.

CSS has some kind of specificity rules (https://developer.mozilla.org/en-US/docs/Web/CSS/Specificity) and I think we could construct a partially ordered set on our conditions language, where we apply a set of rules to determine if two conditions are more or less specific, or not comparable or equal.

Rules:

Property Matches between different properties are never comparable

Example:

• url_param.foo: bar is not comparable to url_param.bar: foo

For substring-starts matching, the longer substring is more specific than the shorter substring.

Examples:

• url: https://www.project-helix.io/ is less specific than url: https://www.project-helix.io/docs/
• url.path: /docs/README.md is more specific than url.path: /docs/
• url.host: www.project-helix.io is not comparable to url.host: project-helix.io
• user_agent: Lynx is equal to user_agent: Lynx

Two strict-match matches are always equal in specificity

Examples:

• client_country_code=: DE is equal to client_country_code=: CH

Two regex-match matches are always equal in specificity

Note: We could (in theory) turn the Regular Expressions into two DFA graphs using https://github.com/hokein/automata.js/ and find out if one graph is a subgraph of the other, in which case it would be less specific.

Examples:

• accept_language~: en is not comparable to accept_language~: en_US

A strict-match is more specific than a substring-starts match, if one contains the other

Examples:

• url.path=: /docs/README.md is more specific than url.path: /docs/
• url.path=: /docs/ is more specific than url.path: /docs/
• url.path=: /docs/ is more specific than url.path: /docs/README.md

A strict-match is more specific than the regex-match it matches

Examples:

• url.path=: /docs/ is more specific than url.path~: /docs/
• url.path=: /docs/ is more specific than url.path~: ^/docs/$
• url.path=: /docs/ is not comparable to url.path~: /frogs/

Property range matches are comparable

Examples:

• client_lat>: 100 is more specific than client_lat>: 99

Two not expressions are comparable if their children are comparable

Examples:

not:
  url.path: /docs/

is comparable to

not:
  url:path: /docs/foo

but not to

not:
  url.path: /foo/docs

Two and or or expressions are comparable if children of one expression are a subset of children of the other expression

Note: this condition can potentially be loosened by taking into account the compatibility and specificity of pairs of children.

Example:

and:
  url.hostname: www.adobe.io
  url.path: /docs

and

and:
  url.hostname: www.adobe.io
  url.path: /docs
  client_country_code: DE

are comparable, but both aren't comparable to:

and:
  accept_language~: de
  client_country_code: DE

Of two comparable or expressions, the one with more children is less specific

or:
  url.hostname: www.adobe.io
  url.path: /docs

is more specific than

or:
  url.hostname: www.adobe.io
  url.path: /docs
  client_country_code: DE

Of two comparable and expressions, the one with more children is more specific

Example:

and:
  url.hostname: www.adobe.io
  url.path: /docs

is less specific than

and:
  url.hostname: www.adobe.io
  url.path: /docs
  client_country_code: DE

Of two comparable and and or expressions, the and expression is more specific

or:
  url.hostname: www.adobe.io
  url.path: /docs

is less specific than

and:
  url.hostname: www.adobe.io
  url.path: /docs

Originally posted by @trieloff in #71 (comment)

While looking at
https://github.com/adobe/developer.adobe.com-planning/issues/172#issuecomment-472738590 I noticed that the performance metrics supported by Helix (Calibre) are not listed in the schema.

see #12

• create json schema for configuration (#25)
• validate json during HelixConfig.parse()
• remove old toplevel defaults
• add tooling to create effective-helix-config.yaml (moved to #28)
• add tooling to edit and write back the helix-config.yaml (moved to #30)
• change helix-cli according to the changes (moved to adobe/helix-cli#449)
• change helix-simulator according to the changes (moved to adobe/helix-simulator#121)

See adobe/developer.adobe.com#144 (comment) – we should communicate breaking changes more clearly.

see #22

It looks like we need to have another discussion about our configuration format and how configuration behaves. What we have at the moment, looks like this:

• users create a helix-config.yaml which will be loaded and denormalized into .hlx/strains.json
• the helix-config.yaml contains a number of top-level (global) definitions and a map of strains
• each defined strain takes default values from:
  • the global definitions
  • the command line options
  • the "default" strain
  • application defaults
  • (the exact precedence rules differ from property to property)

Our developer needs include:

• the ability to start developing without any configuration file at all
• the ability to quickly generate testable strains for code or content branches that are under development
• the ability to quickly and conveniently switch between strains in simulator and production
• the assurance that none of the above will break a published site
• the ability to do all of the above in a fully automated fashion on a CI system (e.g. generate and deploy a new strain for a new branch)

Site administration needs also include:

• the ability to define content mount points
• the ability to pass through traffic to another CMS (proxy strains)

Furthermore, we should establish:

• automated validation of configuration files that is independent from loading them
• detailed and generated documentation for all configuration properties

the jsdom document should be available throughout the pipeline and also available in the response. see adobe/helix-pipeline#337

Is your feature request related to a problem? Please describe.
Examples in CONFIGURATION.md need to be reviewed and updated as necessary.

e.g.

definitions:
  strains:
    base: &basestrain
->      code: /acapt/default/https---github-com-adobe-project-helix-io-git--master--

Describe the solution you'd like
Examples in CONFIGURATION.md should be valid according to the current specification.

Is your feature request related to a problem? Please describe.
This is a follow up issue for adobe/helix-cli#496 (option 3)

Describe the solution you'd like
Strain has a member property of type Static and needs to be aware of deep changes therein so it can recalculate its YAML. Ideally, the Static instance would emit change events which the Strain then could react on.

Describe alternatives you've considered
Alternative solutions discussed in the issue above include calling "internal" methods (1) and deep setters (2), but none are as elegant as using events.

In helix-cli a strain can have following properties:

• url: a base URL for the strain, replaces condition
• urls: a list of URLs (as strings) for hlx perf performance testing
• sticky: a boolean indicating that a strain is sticky or non-sticky, i.e. should result in a cookie or not

None of this is represented in the Strain class.

When an exception or error is passed to the log method, include the stack trace in the log format, if possible.

This one step out adobe/helix-home#5

Since helix-shared is a central piece, it is crucial that it is part of the smoke tests infrastructure.

Code introduced by #71 does not fully support strains "map" (instead of strains "sequence") which was the previous way of defining the list of strains in the helix-config.yaml: if a ref of a GitURL is missing, the schema validation rejects the config, which was not the case before.
A migration (transform "map" to "sequence") is then required. Thus no need to keep the code that was supposed to still support the legacy version. It must be removed.

Description
the order of properties is not guaranteed to be stable in objects, nor in the JSON serialization. this causes the order of strains to be undeterministic. Since the condition matching relies on the order, it is important to keep it stable.

To Reproduce
not so easy, since the object hash is mostly stable.

Version:

$ hlx --version
0.13.11-pre.9

@trieloff should we disallow the / in the directoryIndex property via schema? I think this only leads to confusion.

Originally posted by @tripodsan in adobe/helix-cli#825 (comment)

Empathetically, yes, we should forbid it.

Is your feature request related to a problem? Please describe.
GitUtils uses shell-js to execute git commands to retrieve basic information like remote origin and current branch. this has certain drawbacks:

• only operates on cwd. i.e. the function would need to change the cwd and restore it after executing
• it relies on the existence of a git command
• potentially slower than a native implementation
• potential problems, if the git output changes

advantage:

• no package dependency
• git is very stable and robust, potential implementations might fail

Describe the solution you'd like
Use isomorphic-git (or similar). Since we currently only need a read-only operations, this might be overkill, as it comes with many dependencies. however, we use isomorphic-git also in git-server, so it would already be present in the cli.

Alternative

• find different library
• only use in helix-cli, so that helix-shared stays small

git information that is read during up, deploy, etc:

• isDirty (git status --porcelain)
• getBranch (git rev-parse --abbrev-ref HEAD)
• getRepository (git config --get remote.origin.url)
• getCurrentRevision (git rev-parse HEAD)

@trieloff @dominique-pfister I added an Appendix A to the original description. we might tackle this in a separate issue.

Originally posted by @tripodsan in #20 (comment)

Support to calculate base-url

For conditions that select a strain based on URL, it is important to determine the common root path of the URL that matched the request. this base-url acts as chroot for resource processing.

Examples:

(the regexp support might be an advanced feature...)

So it might be better to return a ConditionResult from toFunction()(req) that contains some method to determine the base url. eg:

const res = strain.getCondition().match(req);
if (res) {
  const baseUrl = res.getBaseUrl();
  ...
}

for VCL, the condition should be able to produce a script that calculates the baseURL.

when adding a property to a strain with merged properties,

  clone:
    <<: *basestrain
    url: https://www.test.com/
    code: https://github.com/adobe/helix-test.git#master

...the merge key gets lost:

  clone:
    url: https://www.test.com/
    code: https://github.com/adobe/helix-test.git#master
    package: bfbde5fbfbde5fbfbde5f

    const src = 'http://localhost/owner/repo.git#local_path';
    const original = new GitUrl(src);
    const result = new GitUrl(original);
    assert.equal(result.toString(), src);

fails with wrong port 443

Description
apparently, the position of comments can confuse the yaml parser.
see https://github.com/adobe/developer.adobe.com/commit/3624692853fa66cd907cbbdc3b7268adbc1ec02c

To Reproduce
see commit above.

Additional Context
according to https://yaml.org/spec/1.2/spec.html#id2780069,

Outside scalar content, comments may appear on a line of their own, independent of the indentation level. Note that outside scalar content, a line containing only white space characters is taken to be a comment line.

/cc @filmaj

For deploy and publish, one should be sure what the effective configuration values are and where they originate from. the ideas is to create an effective-helix-config.yaml (or print one to stdout) that contains annotations (comments) that describe the source of a configuration value.