You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@taverna.apache.org by ThilinaManamgoda <gi...@git.apache.org> on 2017/04/08 21:54:10 UTC

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

GitHub user ThilinaManamgoda opened a pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24

    Resolve imports

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ThilinaManamgoda/incubator-taverna-common-activities resolve-imports

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-taverna-common-activities/pull/24.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #24
    
----
commit 5c2cf7408e23dea15c75f5565cbe7ae4f85862d5
Author: M.G.T.R Manamgoda <ma...@gmail.com>
Date:   2016-08-26T18:57:03Z

    Update Readme.md

commit 68e400a48f85a9593d49a7921c1334ee9269fe30
Author: ThilinaManamgoda <ma...@gmail.com>
Date:   2016-08-26T19:17:35Z

    setToolName => setName

commit 5b88d92b7def361de6f27c1fd2261ea5b7eb2c50
Author: ThilinaManamgoda <ma...@gmail.com>
Date:   2016-08-26T19:18:58Z

    Merge branch 'cwl-licenses'

commit 7ef27f3bab913147b310d1c4d96a7b0dbdb221a8
Author: M.G.T.R Manamgoda <ma...@gmail.com>
Date:   2016-08-26T19:28:19Z

    Update Readme.md

commit f23830b6c48a70de7d2416b3c8f038690ecdb0d1
Author: M.G.T.R Manamgoda <ma...@gmail.com>
Date:   2016-08-26T19:46:09Z

    Update Readme.md

commit b30832c5c4fa861f80723af508ad0ddb50cb2e79
Author: M.G.T.R Manamgoda <ma...@gmail.com>
Date:   2016-12-20T06:32:18Z

    Merge pull request #1 from ThilinaManamgoda/cwl-licenses
    
    Cwl licenses

commit 4541d895f9d2d065a81922177a7b2ef22f4e8a08
Author: maanadev <ma...@gmail.com>
Date:   2017-04-08T00:56:22Z

    Imports are resolving

commit 6b138fb27673e1aea8b3edffd17c0723c9e69383
Author: maanadev <ma...@gmail.com>
Date:   2017-04-08T21:51:05Z

    Test cases are implemented.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by stain <gi...@git.apache.org>.
Github user stain commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r112029419
  
    --- Diff: taverna-cwl-utilities/src/main/java/org/apache/taverna/cwl/utilities/preprocessing/ImportViaHTTP.java ---
    @@ -0,0 +1,41 @@
    +/*******************************************************************************
    + *  Licensed to the Apache Software Foundation (ASF) under one or more
    + *     contributor license agreements.  See the NOTICE file distributed with
    + *     this work for additional information regarding copyright ownership.
    + *     The ASF licenses this file to You under the Apache License, Version 2.0
    + *     (the "License"); you may not use this file except in compliance with
    + *     the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + *     Unless required by applicable law or agreed to in writing, software
    + *     distributed under the License is distributed on an "AS IS" BASIS,
    + *     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + *     See the License for the specific language governing permissions and
    + *     limitations under the License.
    + *******************************************************************************/
    +package org.apache.taverna.cwl.utilities.preprocessing;
    +
    +import com.fasterxml.jackson.databind.JsonNode;
    +import org.apache.log4j.Logger;
    +
    +import java.io.BufferedInputStream;
    +import java.io.IOException;
    +import java.net.URI;
    +
    +
    +public class ImportViaHTTP implements ImportData {
    +    final private static Logger logger = Logger.getLogger(ImportViaHTTP.class);
    +
    +    @Override
    +    public JsonNode importData(URI uri) {
    +
    +        try (BufferedInputStream inputStream = new BufferedInputStream(uri.toURL().openStream())) {
    --- End diff --
    
    Good, I think this will support Taverna's username/password mechanism if Basic Auth is needed, or SSL certification checks if arbitrary certificates are used.
    
    I think this would however also support _climbing out_ and say import `file:///etc/passwd` 
     or `http://intranet15:1313/` - let's see afterwards how we can be more selective about which hosts are OK to use for imports (e.g. browser-like same-origin checking and UI pop-up to confirm)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by ThilinaManamgoda <gi...@git.apache.org>.
Github user ThilinaManamgoda commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r124124903
  
    --- Diff: taverna-cwl-utilities/src/test/resources/preprocessing/ImportResoultionUtil-processNode-Method/processNode.yaml ---
    @@ -0,0 +1,42 @@
    +################################################################################
    +#  Licensed to the Apache Software Foundation (ASF) under one or more
    +#     contributor license agreements.  See the NOTICE file distributed with
    +#     this work for additional information regarding copyright ownership.
    +#     The ASF licenses this file to You under the Apache License, Version 2.0
    +#     (the "License"); you may not use this file except in compliance with
    +#     the License.  You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +#     Unless required by applicable law or agreed to in writing, software
    +#     distributed under the License is distributed on an "AS IS" BASIS,
    +#     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +#     See the License for the specific language governing permissions and
    +#     limitations under the License.
    +#################################################################################
    +
    +
    +cwlVersion: v1.0
    --- End diff --
    
    I made them for testing.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by stain <gi...@git.apache.org>.
Github user stain commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r112638183
  
    --- Diff: taverna-cwl-utilities/src/test/resources/log4j.properties ---
    @@ -0,0 +1,16 @@
    +# Root logger option
    --- End diff --
    
    Add ASF license header, presumably using `#` as comment


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by stain <gi...@git.apache.org>.
Github user stain commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r112638837
  
    --- Diff: taverna-cwl-activity-ui/pom.xml ---
    @@ -112,6 +112,12 @@
     			<artifactId>jackson-databind</artifactId>
     			<version>${jackson.version}</version>
     		</dependency>
    +		<dependency>
    --- End diff --
    
    SnakeYAML [is Apache License 2.0](https://bitbucket.org/asomov/snakeyaml/src/70abb5efa4c0ec0dd3680383c9fabb5cd3939c80/LICENSE.txt?at=v1.17&fileviewer=file-view-default) - so OK to use. (Why not `1.18`?)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by stain <gi...@git.apache.org>.
Github user stain commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r112638431
  
    --- Diff: taverna-cwl-utilities/src/test/resources/preprocessing/serverContent/import.yaml ---
    @@ -0,0 +1,3 @@
    +hello:
    --- End diff --
    
    ASF license header missing (even if this file is really tiny..)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by ThilinaManamgoda <gi...@git.apache.org>.
Github user ThilinaManamgoda commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r128189464
  
    --- Diff: taverna-cwl-utilities/src/main/java/org/apache/taverna/cwl/utilities/preprocessing/LinkedResolutionUtil.java ---
    @@ -0,0 +1,124 @@
    +/*******************************************************************************
    + *  Licensed to the Apache Software Foundation (ASF) under one or more
    + *     contributor license agreements.  See the NOTICE file distributed with
    + *     this work for additional information regarding copyright ownership.
    + *     The ASF licenses this file to You under the Apache License, Version 2.0
    + *     (the "License"); you may not use this file except in compliance with
    + *     the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + *     Unless required by applicable law or agreed to in writing, software
    + *     distributed under the License is distributed on an "AS IS" BASIS,
    + *     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + *     See the License for the specific language governing permissions and
    + *     limitations under the License.
    + *******************************************************************************/
    +package org.apache.taverna.cwl.utilities.preprocessing;
    +
    +import com.fasterxml.jackson.databind.JsonNode;
    +import org.apache.log4j.Logger;
    +
    +import java.io.File;
    +import java.net.URI;
    +import java.net.URISyntaxException;
    +import java.nio.file.Path;
    +
    +/**
    + * This class is Util class which is used for resolving Linked resolution
    + * Rules are defined at: http://www.commonwl.org/draft-3/SchemaSalad.html#Link_resolution
    --- End diff --
    
    yes they are the same in both versions


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by ThilinaManamgoda <gi...@git.apache.org>.
Github user ThilinaManamgoda commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r124124470
  
    --- Diff: taverna-cwl-utilities/src/main/java/org/apache/taverna/cwl/utilities/preprocessing/ImportNode.java ---
    @@ -0,0 +1,31 @@
    +/*******************************************************************************
    + *  Licensed to the Apache Software Foundation (ASF) under one or more
    --- End diff --
    
    done



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by ThilinaManamgoda <gi...@git.apache.org>.
Github user ThilinaManamgoda commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r124126809
  
    --- Diff: taverna-cwl-activity-ui/pom.xml ---
    @@ -112,6 +112,12 @@
     			<artifactId>jackson-databind</artifactId>
     			<version>${jackson.version}</version>
     		</dependency>
    +		<dependency>
    --- End diff --
    
    version is updated 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities issue #24: Resolve imports

Posted by stain <gi...@git.apache.org>.
Github user stain commented on the issue:

    https://github.com/apache/incubator-taverna-common-activities/pull/24
  
    What are all those changes to .gitignore? Which lines of those are really needed for IntelliJ? They seem to be copy-pasted from elsewhere and I'm not sure if you are able to contribute it directly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by ThilinaManamgoda <gi...@git.apache.org>.
Github user ThilinaManamgoda commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r124124740
  
    --- Diff: taverna-cwl-utilities/src/test/java/org/apache/taverna/cwl/utilities/preprocessing/ImportNodeImplTest.java ---
    @@ -0,0 +1,25 @@
    +
    +
    +/***************************************************************************************************
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + **************************************************************************************************/
    +
    +
    +
    +package org.apache.taverna.cwl.utilities.preprocessing;
    +
    +/**
    + * Created by maanadev on 4/9/17.
    --- End diff --
    
    Comment is removed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by stain <gi...@git.apache.org>.
Github user stain commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r112029624
  
    --- Diff: taverna-cwl-utilities/src/main/java/org/apache/taverna/cwl/utilities/preprocessing/LinkedResolutionUtil.java ---
    @@ -0,0 +1,124 @@
    +/*******************************************************************************
    + *  Licensed to the Apache Software Foundation (ASF) under one or more
    + *     contributor license agreements.  See the NOTICE file distributed with
    + *     this work for additional information regarding copyright ownership.
    + *     The ASF licenses this file to You under the Apache License, Version 2.0
    + *     (the "License"); you may not use this file except in compliance with
    + *     the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + *     Unless required by applicable law or agreed to in writing, software
    + *     distributed under the License is distributed on an "AS IS" BASIS,
    + *     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + *     See the License for the specific language governing permissions and
    + *     limitations under the License.
    + *******************************************************************************/
    +package org.apache.taverna.cwl.utilities.preprocessing;
    +
    +import com.fasterxml.jackson.databind.JsonNode;
    +import org.apache.log4j.Logger;
    +
    +import java.io.File;
    +import java.net.URI;
    +import java.net.URISyntaxException;
    +import java.nio.file.Path;
    +
    +/**
    + * This class is Util class which is used for resolving Linked resolution
    + * Rules are defined at: http://www.commonwl.org/draft-3/SchemaSalad.html#Link_resolution
    --- End diff --
    
    Are they also compatible with http://www.commonwl.org/v1.0/SchemaSalad.html#Link_resolution ? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by ThilinaManamgoda <gi...@git.apache.org>.
Github user ThilinaManamgoda commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r124124377
  
    --- Diff: taverna-cwl-utilities/src/main/java/org/apache/taverna/cwl/utilities/preprocessing/ImportNodeImpl.java ---
    @@ -0,0 +1,33 @@
    +/*******************************************************************************
    + *  Licensed to the Apache Software Foundation (ASF) under one or more
    + *     contributor license agreements.  See the NOTICE file distributed with
    + *     this work for additional information regarding copyright ownership.
    + *     The ASF licenses this file to You under the Apache License, Version 2.0
    + *     (the "License"); you may not use this file except in compliance with
    + *     the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + *     Unless required by applicable law or agreed to in writing, software
    + *     distributed under the License is distributed on an "AS IS" BASIS,
    + *     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + *     See the License for the specific language governing permissions and
    + *     limitations under the License.
    + *******************************************************************************/
    +package org.apache.taverna.cwl.utilities.preprocessing;
    +
    +import com.fasterxml.jackson.databind.JsonNode;
    +
    +import java.net.URI;
    +
    +/**
    + * This class provides abstract between node importing and how it's imported
    + */
    +public class ImportNodeImpl implements ImportNode {
    --- End diff --
    
    It's bit confusing I see it now. Let's keep ImportNode and remove ImportData


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by stain <gi...@git.apache.org>.
Github user stain commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r112027060
  
    --- Diff: taverna-cwl-utilities/src/main/java/org/apache/taverna/cwl/utilities/preprocessing/ImportData.java ---
    @@ -0,0 +1,26 @@
    +package org.apache.taverna.cwl.utilities.preprocessing;
    --- End diff --
    
    Could you add a license header here?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by ThilinaManamgoda <gi...@git.apache.org>.
Github user ThilinaManamgoda commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r124124432
  
    --- Diff: taverna-cwl-utilities/src/main/java/org/apache/taverna/cwl/utilities/preprocessing/CwlPreprocessor.java ---
    @@ -0,0 +1,39 @@
    +/*******************************************************************************
    + *  Licensed to the Apache Software Foundation (ASF) under one or more
    --- End diff --
    
    done



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by ThilinaManamgoda <gi...@git.apache.org>.
Github user ThilinaManamgoda commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r128213983
  
    --- Diff: taverna-cwl-utilities/src/main/java/org/apache/taverna/cwl/utilities/preprocessing/LinkedResolutionUtil.java ---
    @@ -0,0 +1,124 @@
    +/*******************************************************************************
    + *  Licensed to the Apache Software Foundation (ASF) under one or more
    + *     contributor license agreements.  See the NOTICE file distributed with
    + *     this work for additional information regarding copyright ownership.
    + *     The ASF licenses this file to You under the Apache License, Version 2.0
    + *     (the "License"); you may not use this file except in compliance with
    + *     the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + *     Unless required by applicable law or agreed to in writing, software
    + *     distributed under the License is distributed on an "AS IS" BASIS,
    + *     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + *     See the License for the specific language governing permissions and
    + *     limitations under the License.
    + *******************************************************************************/
    +package org.apache.taverna.cwl.utilities.preprocessing;
    +
    +import com.fasterxml.jackson.databind.JsonNode;
    +import org.apache.log4j.Logger;
    +
    +import java.io.File;
    +import java.net.URI;
    +import java.net.URISyntaxException;
    +import java.nio.file.Path;
    +
    +/**
    + * This class is Util class which is used for resolving Linked resolution
    + * Rules are defined at: http://www.commonwl.org/draft-3/SchemaSalad.html#Link_resolution
    + */
    +public class LinkedResolutionUtil implements CwlPreprocessor {
    +
    +    final private static Logger logger = Logger.getLogger(LinkedResolutionUtil.class);
    +
    +    private JsonNode cwlToolDescription;
    +    private JsonNode nameSpace;
    +    private URI BASE;
    +    private Path path;
    +
    +    /**
    +     *
    +     * @param cwlToolDescription CWL tool description
    +     * @param path This must be the directory where CWL tool is located and resolving is done assuming required other files
    +     *             in the same directory
    +     */
    +    public LinkedResolutionUtil(JsonNode cwlToolDescription, Path path) {
    +        this.cwlToolDescription = cwlToolDescription;
    +        this.path = path;
    +        setup();
    +    }
    +
    +    private void setBASE(URI BASE) {
    +        this.BASE = BASE;
    +    }
    +
    +    /**
    +     * This method setup the initial resources for the process
    +     */
    +    private void setup() {
    +        try {
    +            if (cwlToolDescription.has("$base"))
    +                setBASE(new URI(cwlToolDescription.get("$base").asText()));
    +            if (cwlToolDescription.has("$namespaces"))
    +                setNameSpace(cwlToolDescription.get("$namespaces"));
    +        } catch (URISyntaxException e) {
    +            logger.error("Setup function ", e);
    +        }
    +    }
    +
    +
    +    private void setNameSpace(JsonNode nameSpace) {
    +        this.nameSpace = nameSpace;
    +    }
    +
    +    /**
    +     * This method is resolving the uri based on the rules mentioned in the CWL Schema Salad
    +     * @param val This is a ValueNode (subclass of a JsonNode) which holds the link. This must be a single node
    +     * @return An URI object which contains absolute is returned
    +     * @throws URISyntaxException
    +     */
    +    @Override
    +    public URI process(JsonNode val) throws URISyntaxException {
    +
    +        String uriString = val.asText();
    +        URI uri = new URI(uriString);
    +
    +        if (uri.isAbsolute()) {
    --- End diff --
    
    I fixed it to some extend but i don't think i understand the problem properly. Can you please explain little bit more


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities issue #24: Resolve imports

Posted by ThilinaManamgoda <gi...@git.apache.org>.
Github user ThilinaManamgoda commented on the issue:

    https://github.com/apache/incubator-taverna-common-activities/pull/24
  
    Sorry, my mistake. It's fixed now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by ThilinaManamgoda <gi...@git.apache.org>.
Github user ThilinaManamgoda commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r124124483
  
    --- Diff: taverna-cwl-utilities/src/main/java/org/apache/taverna/cwl/utilities/preprocessing/ImportData.java ---
    @@ -0,0 +1,26 @@
    +package org.apache.taverna.cwl.utilities.preprocessing;
    --- End diff --
    
    done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by stain <gi...@git.apache.org>.
Github user stain commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r112030157
  
    --- Diff: taverna-cwl-utilities/src/main/java/org/apache/taverna/cwl/utilities/preprocessing/LinkedResolutionUtil.java ---
    @@ -0,0 +1,124 @@
    +/*******************************************************************************
    + *  Licensed to the Apache Software Foundation (ASF) under one or more
    + *     contributor license agreements.  See the NOTICE file distributed with
    + *     this work for additional information regarding copyright ownership.
    + *     The ASF licenses this file to You under the Apache License, Version 2.0
    + *     (the "License"); you may not use this file except in compliance with
    + *     the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + *     Unless required by applicable law or agreed to in writing, software
    + *     distributed under the License is distributed on an "AS IS" BASIS,
    + *     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + *     See the License for the specific language governing permissions and
    + *     limitations under the License.
    + *******************************************************************************/
    +package org.apache.taverna.cwl.utilities.preprocessing;
    +
    +import com.fasterxml.jackson.databind.JsonNode;
    +import org.apache.log4j.Logger;
    +
    +import java.io.File;
    +import java.net.URI;
    +import java.net.URISyntaxException;
    +import java.nio.file.Path;
    +
    +/**
    + * This class is Util class which is used for resolving Linked resolution
    + * Rules are defined at: http://www.commonwl.org/draft-3/SchemaSalad.html#Link_resolution
    + */
    +public class LinkedResolutionUtil implements CwlPreprocessor {
    +
    +    final private static Logger logger = Logger.getLogger(LinkedResolutionUtil.class);
    +
    +    private JsonNode cwlToolDescription;
    +    private JsonNode nameSpace;
    +    private URI BASE;
    +    private Path path;
    +
    +    /**
    +     *
    +     * @param cwlToolDescription CWL tool description
    +     * @param path This must be the directory where CWL tool is located and resolving is done assuming required other files
    +     *             in the same directory
    +     */
    +    public LinkedResolutionUtil(JsonNode cwlToolDescription, Path path) {
    +        this.cwlToolDescription = cwlToolDescription;
    +        this.path = path;
    +        setup();
    +    }
    +
    +    private void setBASE(URI BASE) {
    +        this.BASE = BASE;
    +    }
    +
    +    /**
    +     * This method setup the initial resources for the process
    +     */
    +    private void setup() {
    +        try {
    +            if (cwlToolDescription.has("$base"))
    +                setBASE(new URI(cwlToolDescription.get("$base").asText()));
    +            if (cwlToolDescription.has("$namespaces"))
    +                setNameSpace(cwlToolDescription.get("$namespaces"));
    +        } catch (URISyntaxException e) {
    +            logger.error("Setup function ", e);
    +        }
    +    }
    +
    +
    +    private void setNameSpace(JsonNode nameSpace) {
    +        this.nameSpace = nameSpace;
    +    }
    +
    +    /**
    +     * This method is resolving the uri based on the rules mentioned in the CWL Schema Salad
    +     * @param val This is a ValueNode (subclass of a JsonNode) which holds the link. This must be a single node
    +     * @return An URI object which contains absolute is returned
    +     * @throws URISyntaxException
    +     */
    +    @Override
    +    public URI process(JsonNode val) throws URISyntaxException {
    +
    +        String uriString = val.asText();
    +        URI uri = new URI(uriString);
    +
    +        if (uri.isAbsolute()) {
    --- End diff --
    
    Not sure if this is the right location, but how can the imports support relative URIs based on the URI of the CWL document that the `$import` node comes from?
    
    e.g.
    
    `http://example.com/a/b/c.cwl` imports `../sibling/e.cwl` (aka `http://example.com/a/sibling/e.cwl`) which then imports `f.cwl` (aka `http://example.com/a/sibling/f.cwl` - not ``http://example.com/a/b/f.cwl`)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by ThilinaManamgoda <gi...@git.apache.org>.
Github user ThilinaManamgoda commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r124127094
  
    --- Diff: taverna-cwl-utilities/src/test/java/org/apache/taverna/cwl/utilities/preprocessing/ImportResolutionUtilTest.java ---
    @@ -0,0 +1,90 @@
    +/***************************************************************************************************
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + **************************************************************************************************/
    +
    +package org.apache.taverna.cwl.utilities.preprocessing;
    +
    +import com.fasterxml.jackson.databind.JsonNode;
    +import org.junit.*;
    +
    +import java.io.File;
    +import java.io.IOException;
    +import java.net.URISyntaxException;
    +import java.nio.file.Paths;
    +
    +import static org.apache.taverna.cwl.utilities.ImportNodeViaFile.getNode;
    +import static org.junit.Assert.assertEquals;
    +
    +public class ImportResolutionUtilTest {
    +    static Process exec;
    +    JsonNode localFile, localFileResult, fileOverHttPWithFragment, fileOverHttPWithFragmentResult,
    +            fileOverHttpWithNamespace, fileOverHttpWithNamespaceResult, testProcessNode, testProcessNodeResult;
    +
    +    @BeforeClass
    +    public static void setUpHttPServer() throws IOException {
    +        String cmd[] = {"python", "-m", "SimpleHTTPServer", "8000"};
    --- End diff --
    
    Now Jetty server with a random port is used for testing


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by stain <gi...@git.apache.org>.
Github user stain commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r112638360
  
    --- Diff: taverna-cwl-utilities/src/test/resources/preprocessing/ImportResoultionUtil-processNode-Method/processNode.yaml ---
    @@ -0,0 +1,42 @@
    +################################################################################
    +#  Licensed to the Apache Software Foundation (ASF) under one or more
    +#     contributor license agreements.  See the NOTICE file distributed with
    +#     this work for additional information regarding copyright ownership.
    +#     The ASF licenses this file to You under the Apache License, Version 2.0
    +#     (the "License"); you may not use this file except in compliance with
    +#     the License.  You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +#     Unless required by applicable law or agreed to in writing, software
    +#     distributed under the License is distributed on an "AS IS" BASIS,
    +#     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +#     See the License for the specific language governing permissions and
    +#     limitations under the License.
    +#################################################################################
    +
    +
    +cwlVersion: v1.0
    --- End diff --
    
    What's the origin of these test workflows? Did you make them, or did they come from the `commonwl.org` site?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by stain <gi...@git.apache.org>.
Github user stain commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r112638582
  
    --- Diff: .gitignore ---
    @@ -8,3 +8,101 @@ Testing.java
     # remove emacs
     *~
     
    +
    +# Created by https://www.gitignore.io/api/intellij,intellij+iml
    --- End diff --
    
    What's the license of the output of gitignore.io? Also I don't think we really need all these ignores for intelliJ -- would not a one line ignore of `.idea/` be sufficient?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by ThilinaManamgoda <gi...@git.apache.org>.
Github user ThilinaManamgoda commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r124125308
  
    --- Diff: taverna-cwl-utilities/src/test/resources/preprocessing/serverContent/import.yaml ---
    @@ -0,0 +1,3 @@
    +hello:
    --- End diff --
    
    done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by stain <gi...@git.apache.org>.
Github user stain commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r112030778
  
    --- Diff: taverna-cwl-utilities/src/test/java/org/apache/taverna/cwl/utilities/preprocessing/ImportNodeImplTest.java ---
    @@ -0,0 +1,25 @@
    +
    +
    +/***************************************************************************************************
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + **************************************************************************************************/
    +
    +
    +
    +package org.apache.taverna.cwl.utilities.preprocessing;
    +
    +/**
    + * Created by maanadev on 4/9/17.
    --- End diff --
    
    I suggest to  skip "Created by XXX" comments - and the date (which date format is it even?).
    
    However it was discussed in a resurrection of [TAVERNA-897](https://issues.apache.org/jira/browse/TAVERNA-897) to still recognize individual authors in comments as git logs are fragile over time.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by stain <gi...@git.apache.org>.
Github user stain commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r112027115
  
    --- Diff: taverna-cwl-utilities/src/main/java/org/apache/taverna/cwl/utilities/preprocessing/ImportNode.java ---
    @@ -0,0 +1,31 @@
    +/*******************************************************************************
    + *  Licensed to the Apache Software Foundation (ASF) under one or more
    --- End diff --
    
    Nitpick: 
    indentation error in header. This seems to be true for all of your added files.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by stain <gi...@git.apache.org>.
Github user stain commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r112637921
  
    --- Diff: taverna-cwl-utilities/src/test/java/org/apache/taverna/cwl/utilities/preprocessing/ImportResolutionUtilTest.java ---
    @@ -0,0 +1,90 @@
    +/***************************************************************************************************
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + **************************************************************************************************/
    +
    +package org.apache.taverna.cwl.utilities.preprocessing;
    +
    +import com.fasterxml.jackson.databind.JsonNode;
    +import org.junit.*;
    +
    +import java.io.File;
    +import java.io.IOException;
    +import java.net.URISyntaxException;
    +import java.nio.file.Paths;
    +
    +import static org.apache.taverna.cwl.utilities.ImportNodeViaFile.getNode;
    +import static org.junit.Assert.assertEquals;
    +
    +public class ImportResolutionUtilTest {
    +    static Process exec;
    +    JsonNode localFile, localFileResult, fileOverHttPWithFragment, fileOverHttPWithFragmentResult,
    +            fileOverHttpWithNamespace, fileOverHttpWithNamespaceResult, testProcessNode, testProcessNodeResult;
    +
    +    @BeforeClass
    +    public static void setUpHttPServer() throws IOException {
    +        String cmd[] = {"python", "-m", "SimpleHTTPServer", "8000"};
    --- End diff --
    
    Why is this using Python to run a HTTP server? Can we not use Jetty programmatically?
    
    Note that port `8000` might not be free, so this port needs to be bound dynamically (And then `8000` search-replaced in the rest of the test)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by ThilinaManamgoda <gi...@git.apache.org>.
Github user ThilinaManamgoda commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r124124802
  
    --- Diff: taverna-cwl-utilities/src/test/resources/log4j.properties ---
    @@ -0,0 +1,16 @@
    +# Root logger option
    --- End diff --
    
    done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by stain <gi...@git.apache.org>.
Github user stain commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r112027630
  
    --- Diff: taverna-cwl-utilities/src/main/java/org/apache/taverna/cwl/utilities/preprocessing/ImportNodeImpl.java ---
    @@ -0,0 +1,33 @@
    +/*******************************************************************************
    + *  Licensed to the Apache Software Foundation (ASF) under one or more
    + *     contributor license agreements.  See the NOTICE file distributed with
    + *     this work for additional information regarding copyright ownership.
    + *     The ASF licenses this file to You under the Apache License, Version 2.0
    + *     (the "License"); you may not use this file except in compliance with
    + *     the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + *     Unless required by applicable law or agreed to in writing, software
    + *     distributed under the License is distributed on an "AS IS" BASIS,
    + *     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + *     See the License for the specific language governing permissions and
    + *     limitations under the License.
    + *******************************************************************************/
    +package org.apache.taverna.cwl.utilities.preprocessing;
    +
    +import com.fasterxml.jackson.databind.JsonNode;
    +
    +import java.net.URI;
    +
    +/**
    + * This class provides abstract between node importing and how it's imported
    + */
    +public class ImportNodeImpl implements ImportNode {
    --- End diff --
    
    Could you explain the reasoning for `ImportNode` vs `ImportData`?  I don't think it's wrong, just a bit confused.. is it in a way so that the `ImportNode` is kind of the instruction to import, and `ImportData` the imported data?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-taverna-common-activities pull request #24: Resolve imports

Posted by stain <gi...@git.apache.org>.
Github user stain commented on a diff in the pull request:

    https://github.com/apache/incubator-taverna-common-activities/pull/24#discussion_r112027195
  
    --- Diff: taverna-cwl-utilities/src/main/java/org/apache/taverna/cwl/utilities/preprocessing/CwlPreprocessor.java ---
    @@ -0,0 +1,39 @@
    +/*******************************************************************************
    + *  Licensed to the Apache Software Foundation (ASF) under one or more
    --- End diff --
    
    indentation error in header :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---