xmlsquad / xml-authoring-library

A place to put code that is reused by many commands which are part of the [xml-authoring-tools suite](https://github.com/forikal-uk/xml-authoring-tools).
Apache License 2.0
0 stars 0 forks source link

Unify config options, rules to allow all Xml Authoring Command to have unified interface. #4

Open forikal-uk opened 6 years ago

forikal-uk commented 6 years ago

As a user of an Xml Authoring Project, I can set one config file with my Google API connection settings, key client file locations, etc, so that one config file in the project can be used for most Commands.


Notes:

Multiple developers create the commands at the same time, so now is the time to refactor to deduplicate implementations and ensure a consistent interface for the user, where possible, across the suite of tools.

forikal-uk commented 6 years ago

The pattern:

If the user provides:

We assume OAuth authentication with google. Not services. This is to ensure that access is granted on a user-by-user basis and not granted on a tool-by-tool basis. Some users should be prevented access to some GSuite data.

Settings Rules Pseudocode:

client-secret-file

findCredentialsPathOption

if none findCredentialsPathConfigSetting

if none, error.

access-token-file

findAccessTokenFileOption

if none findAccessTokenFileConfigSetting

if none, use default AccessTokenFile.

findXConfigSetting

findConfigFilenameOption

if found set configFilename to findConfigFilenameOption else use default configFilename

findConfigFile

if not found

error.

findSetting

if found return value, else return null.

If would also be handy if client-secret-file and access-token-file could be absolute as well as their current relative paths. Absolute paths would allow the user to run a command from anywhere in the tree without having to hack at the new relative path.

forikal-uk commented 6 years ago

 Update: This comment has been superseded by an updated one below.

----- OLD ------

Although these really ought to be in Behat, let's have make a test cheat sheet to help work on all projects at the same time:

capture-lookups

Phpunit

Currently breaking cos it expects a credentials file.

$ pwd
/Users/jw/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/capture-lookups/tests/

User test

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ rm -f "Testing Public Sheet-Test Sheet2 Funky Name.csv"; rm -f Testing\ Public\ Sheet-Test\ Sheet1.csv; bin/console capture-lookups  --credentials="../../../../APIKeys/gsheet-to-xml-e5eaf708560f.json" --sheet=TestingSheet --force; rm -f Testing\ Public\ Sheet-Test\ Sheet1.csv; rm -f "Testing Public Sheet-Test Sheet2 Funky Name.csv"

gsheet-to-xml

PHPunit

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/gsheet-to-xml/tests/
PHPUnit 7.2.6 by Sebastian Bergmann and contributors.

User test

$ pwd
/Users/jw/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ bin/gsheet-to-xml.php --recursive  --client-secret-file="../../../../APIKeys/OAuth-client_secret_596511451672-flbjqooi8bemek6b96v1oq61jf414oja.apps.googleusercontent.com.json" --access-token-file="delete-me-tokenfile" https://docs.google.com/spreadsheets/d/1hdKksm6Xj6SiL3r8paCzlW2gBMRnm445ZBflUL-591M/edit#gid=311423522

ping-drive

phpunit

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/ping-drive/tests/

User Test

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ bin/console ping-drive  --client-secret-file="../../../../APIKeys/OAuth-client_secret_596511451672-flbjqooi8bemek6b96v1oq61jf414oja.apps.googleusercontent.com.json" --access-token-file="delete-me-tokenfile" https://docs.google.com/spreadsheets/d/1hdKksm6Xj6SiL3r8paCzlW2gBMRnm445ZBflUL-591M/edit#gid=311423522

### xml-authoring-library

#### Phpunit Tests

```bash
$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/xml-authoring-library/tests/

xmlauthor-example-command

Phpunit

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/xmlauthor-example-command/tests/

Currently breaks if no scapesettings.yaml.dist is set in working directory where tests are invoked.

User test

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ cp vendor/xmlsquad/xmlauthor-example-command/scapesettings.yaml.dist scapesettings.yaml; bin/console hello-world ; rm HelloWorld.txt 
[HelloWorld.txt] successfully written.

xml-authoring-library

PHPunit

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/xml-authoring-library/tests/
forikal-uk commented 6 years ago

I guess I can group all those phpunit tests into a test config.

https://phpunit.readthedocs.io/en/7.1/textui.html

Let's create one.

Done.

Now the ones that work can be run:

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit -c phpunit.xml.dist
PHPUnit 7.2.6 by Sebastian Bergmann and contributors.

..............................................................    62 / 62 (100%)

Time: 472 ms, Memory: 10.00MB

OK (62 tests, 1266 assertions)

or more succintly, because phpunit looks for the phpunit.xml.dist by default

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit
PHPUnit 7.2.6 by Sebastian Bergmann and contributors.

..............................................................    62 / 62 (100%)

Time: 472 ms, Memory: 10.00MB

OK (62 tests, 1266 assertions)
forikal-uk commented 6 years ago

I have added a copy of the credentials_test.json to get the capture-looups test working too. It is a bit naughty. But, nice to see them working.

$  ./vendor/phpunit/phpunit/phpunit
PHPUnit 7.2.6 by Sebastian Bergmann and contributors.

................................................................. 65 / 65 (100%)

Time: 4.04 seconds, Memory: 12.00MB

OK (65 tests, 1290 assertions)
forikal-uk commented 6 years ago

Let's commit.

forikal-uk commented 6 years ago

As I was doing 034b85c (Rename config option name), I realised that it might be better if config options were in camelCase and not separated by hyphens. The former would match the variable names and carry through consistency throughout. I will add the to my todo list for later.

forikal-uk commented 6 years ago

Let's make Ping Drive extend our libraries abstract command so I can move methods up to it.

forikal-uk commented 6 years ago

Next. Make ping-drive use the configuration and getters for GApiConnectionOptions.

forikal-uk commented 6 years ago

Wrap up client-secret-file

forikal-uk commented 6 years ago

I am reminded that capture-lookups uses a different auth strategy.

screen shot 2018-07-01 at 22 01 59

So, the question is: "Do we use the same option name for both strategies or have one for Oath and one for ServiceAccountKey?"

It only matters if we have to use both at the same time.

I think we might, at some point, configure both in the config file.

Also, it is easier to merge them in the future (even programatically) than to split them.

So, I will keep them separate but will bring up the capture-lookups api key config into the abstract to guide other commands being made for the suite of tools.

forikal-uk commented 6 years ago

I closed the issue by accident. Re-opened.

forikal-uk commented 6 years ago

Ok. Let's rename the OAuth key name to ensure it is consistent.

I think I will start to introduce camelCase naming to keep it consistent with variable names in the code.

forikal-uk commented 6 years ago

Rename:

client-secret-file

to

gApiOAuthSecretFile

and

GApiSecretFileOption

to

GApiOAuthSecretFileOption

and

rename

clientSecretFile

to

gApiOAuthSecretFile

forikal-uk commented 6 years ago

I have the urge to go renaming all options now. Before I do so, let's check if there is a Symfony standard that defines how to name option arguments.

I couldn't find any.

But, I did find this example page which indicates camelCase in an example command:

-fcWorld -b Hello

So, that makes me happy.

forikal-uk commented 6 years ago

Ok let's do the rename of all options before I naming getter methods and config methods.

forikal-uk commented 6 years ago

accessTokenFile to gApiAccessTokenFile

access-token-file to gApiAccessTokenFile

force-authenticate to forceAuthenticate

drive-url to driveUrl

forikal-uk commented 6 years ago

New updated cheatsheet see below.

Update our cheat sheet


Although these really ought to be in Behat, let's have make a test cheat sheet to help work on all projects at the same time:

capture-lookups

Phpunit

Currently breaking cos it expects a credentials file.

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/capture-lookups/tests/

User test

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ rm -f "Testing Public Sheet-Test Sheet2 Funky Name.csv"; rm -f Testing\ Public\ Sheet-Test\ Sheet1.csv; bin/console capture-lookups  --credentials="../../../../APIKeys/gsheet-to-xml-e5eaf708560f.json" --sheet=TestingSheet --force; rm -f Testing\ Public\ Sheet-Test\ Sheet1.csv; rm -f "Testing Public Sheet-Test Sheet2 Funky Name.csv"

gsheet-to-xml

PHPunit

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/gsheet-to-xml/tests/
PHPUnit 7.2.6 by Sebastian Bergmann and contributors.

User test

$ pwd
/Users/jw/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ bin/gsheet-to-xml.php --recursive  --gApiOAuthSecretFile="../../../../APIKeys/OAuth-client_secret_596511451672-flbjqooi8bemek6b96v1oq61jf414oja.apps.googleusercontent.com.json" --gApiAccessTokenFile="delete-me-tokenfile" https://docs.google.com/spreadsheets/d/1hdKksm6Xj6SiL3r8paCzlW2gBMRnm445ZBflUL-591M/edit#gid=311423522

ping-drive

phpunit

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/ping-drive/tests/

User Test

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ bin/console ping-drive  --gApiOAuthSecretFile="../../../../APIKeys/OAuth-client_secret_596511451672-flbjqooi8bemek6b96v1oq61jf414oja.apps.googleusercontent.com.json" --gApiAccessTokenFile="delete-me-tokenfile" https://docs.google.com/spreadsheets/d/1hdKksm6Xj6SiL3r8paCzlW2gBMRnm445ZBflUL-591M/edit#gid=311423522

### xml-authoring-library

#### Phpunit Tests

```bash
$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/xml-authoring-library/tests/

xmlauthor-example-command

Phpunit

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/xmlauthor-example-command/tests/

Currently breaks if no scapesettings.yaml.dist is set in working directory where tests are invoked.

User test

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ cp vendor/xmlsquad/xmlauthor-example-command/scapesettings.yaml.dist scapesettings.yaml; bin/console hello-world ; rm HelloWorld.txt 
[HelloWorld.txt] successfully written.

xml-authoring-library

PHPunit

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/xml-authoring-library/tests/
forikal-uk commented 6 years ago

Next, continue to move up the config and option getters into the abstract.

forikal-uk commented 6 years ago

Let's also rename the credentials file used by capture-lookups into our format.

To distinguish it from the OAuth credentials.

Rename: credentials to gApiServiceAccountCredentialsFile

forikal-uk commented 6 years ago

Update the test cheat sheet


Update our cheat sheet


Although these really ought to be in Behat, let's have make a test cheat sheet to help work on all projects at the same time:

capture-lookups

Phpunit

Currently breaking cos it expects a gApiServiceAccountCredentialsFile file.

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/capture-lookups/tests/

User test

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ rm -f "Testing Public Sheet-Test Sheet2 Funky Name.csv"; rm -f Testing\ Public\ Sheet-Test\ Sheet1.csv; bin/console capture-lookups  --gApiServiceAccountCredentialsFile="../../../../APIKeys/gsheet-to-xml-e5eaf708560f.json" --sheet=TestingSheet --force; rm -f Testing\ Public\ Sheet-Test\ Sheet1.csv; rm -f "Testing Public Sheet-Test Sheet2 Funky Name.csv"

gsheet-to-xml

PHPunit

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/gsheet-to-xml/tests/
PHPUnit 7.2.6 by Sebastian Bergmann and contributors.

User test

$ pwd
/Users/jw/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ bin/gsheet-to-xml.php --recursive  --gApiOAuthSecretFile="../../../../APIKeys/OAuth-client_secret_596511451672-flbjqooi8bemek6b96v1oq61jf414oja.apps.googleusercontent.com.json" --gApiAccessTokenFile="delete-me-tokenfile" https://docs.google.com/spreadsheets/d/1hdKksm6Xj6SiL3r8paCzlW2gBMRnm445ZBflUL-591M/edit#gid=311423522

ping-drive

phpunit

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/ping-drive/tests/

User Test

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ bin/console ping-drive  --gApiOAuthSecretFile="../../../../APIKeys/OAuth-client_secret_596511451672-flbjqooi8bemek6b96v1oq61jf414oja.apps.googleusercontent.com.json" --gApiAccessTokenFile="delete-me-tokenfile" https://docs.google.com/spreadsheets/d/1hdKksm6Xj6SiL3r8paCzlW2gBMRnm445ZBflUL-591M/edit#gid=311423522

### xml-authoring-library

#### Phpunit Tests

```bash
$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/xml-authoring-library/tests/

xmlauthor-example-command

Phpunit

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/xmlauthor-example-command/tests/

Currently breaks if no scapesettings.yaml.dist is set in working directory where tests are invoked.

User test

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ cp vendor/xmlsquad/xmlauthor-example-command/scapesettings.yaml.dist scapesettings.yaml; bin/console hello-world ; rm HelloWorld.txt 
[HelloWorld.txt] successfully written.

xml-authoring-library

PHPunit

$ pwd
/Users/x/Documents/Projects/XmlAuthoringSuite/xml-authoring-project
$ ./vendor/phpunit/phpunit/phpunit vendor/xmlsquad/xml-authoring-library/tests/
forikal-uk commented 6 years ago

Make capture lookups command extend the xml squad abstract command.

So, it can reuse its config and getter methods.

forikal-uk commented 6 years ago

I am going to rename

doGetGApiOAuthSecretFileOption

to

getGApiOAuthSecretFileOption

and

doConfigureGApiOAuthSecretFileOption

to

configureGApiOAuthSecretFileOption

because these are not true Template Methods.

forikal-uk commented 6 years ago

Next I want to bring up the behaviour, in ping-drive, that uses the config file to store credentials. This will make the user's life easier.

forikal-uk commented 6 years ago

First let's rename the config file

from

scapesettings.yaml.dist

to

XmlAuthoringProjectSettings.yaml.dist

from

scapesettings.yaml

to

XmlAuthoringProjectSettings.yaml

forikal-uk commented 6 years ago

Actually first I gotta pull up gApiAccessTokenFile config and option getter to abstract.

forikal-uk commented 6 years ago

Do same for forceAuthenticate

forikal-uk commented 6 years ago

Pull up gApi config / option setters from ping to abstract

forikal-uk commented 6 years ago

Now let's get the gsheet-to-xml command able to use the config file settings for authentication.

forikal-uk commented 6 years ago

One last thing I need to do before I have a break is to rename the root config key

from:

google

to

gApiAccess

forikal-uk commented 6 years ago

Ok let's tag all these repos cos the interface option names have changed.

Let's call them all v0.3.0-dev.