Merge pull request #337 from overture-stack/develop

Update master to latest develop.

Author: andricDu

Diff for: .circleci/config.yml

@@ -17,16 +17,16 @@ jobs:
       # Download and cache dependencies
       - restore_cache:
           keys:
-          - v1-dependencies-{{ checksum "pom.xml" }}
+          - v2-dependencies-{{ checksum "pom.xml" }}
           # fallback to using the latest cache if no exact match is found
-          - v1-dependencies-
+          - v2-dependencies-
       - run:
           name: Fetch Maven Dependencies
           command: mvn dependency:go-offline
       - save_cache:
           paths:
             - ~/.m2
-          key: v1-dependencies-{{ checksum "pom.xml" }}
+          key: v2-dependencies-{{ checksum "pom.xml" }}
 
       # Run Tests
       - run:
@@ -35,6 +35,36 @@ jobs:
             mvn test \
             -D spring.profiles.active=test
 
+  build-docker-image:
+    machine: true
+
+    steps:
+      - checkout
+
+      - run: docker login -u $DOCKER_USER -p $DOCKER_PASS
+
+      - run: docker build -f Dockerfile.prod . -t overture/ego:$(git describe --always)-alpine
+
+      - run: docker push overture/ego:$(git describe --always)-alpine
+
+  deploy-staging:
+    machine: true
+
+    steps:
+      - checkout
+
+      - run: mkdir ~/.kube && echo $KUBE_CONFIG | base64 --decode > ~/.kube/config
+
+      - run: wget https://storage.googleapis.com/kubernetes-helm/helm-v2.12.1-linux-amd64.tar.gz
+
+      - run: tar -xvf helm-v2.12.1-linux-amd64.tar.gz
+
+      - run: linux-amd64/helm init --client-only
+
+      - run: linux-amd64/helm repo add overture https://overture-stack.github.io/charts/
+
+      - run: linux-amd64/helm upgrade ego-staging overture/ego --reuse-values --set image.tag=$(git describe --always)-alpine
+
   deploy:
     docker:
       - image: circleci/openjdk:8-jdk
@@ -78,10 +108,9 @@ jobs:
           name: Deploy Script
           command: ./.circleci/deploy.sh
 
-
 workflows:
   version: 2
-  test-deploy:
+  test-build-deploy:
     jobs:
       - test
       - deploy:
@@ -91,4 +120,11 @@ workflows:
             branches:
               only:
                 - develop
-                
+      - build-docker-image:
+          filters:
+            branches:
+              only:
+                - develop
+      - deploy-staging:
+          requires:
+            - build-docker-image

Diff for: .gitignore

@@ -38,3 +38,7 @@ _build/
 _source/
 _templates/
 .DS_Store
+
+classes/
+
+local.log

Diff for: .mvn/wrapper/MavenWrapperDownloader.java

@@ -0,0 +1,110 @@
+/*
+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.
+*/
+
+import java.net.*;
+import java.io.*;
+import java.nio.channels.*;
+import java.util.Properties;
+
+public class MavenWrapperDownloader {
+
+    /**
+     * Default URL to download the maven-wrapper.jar from, if no 'downloadUrl' is provided.
+     */
+    private static final String DEFAULT_DOWNLOAD_URL =
+            "https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.4.2/maven-wrapper-0.4.2.jar";
+
+    /**
+     * Path to the maven-wrapper.properties file, which might contain a downloadUrl property to
+     * use instead of the default one.
+     */
+    private static final String MAVEN_WRAPPER_PROPERTIES_PATH =
+            ".mvn/wrapper/maven-wrapper.properties";
+
+    /**
+     * Path where the maven-wrapper.jar will be saved to.
+     */
+    private static final String MAVEN_WRAPPER_JAR_PATH =
+            ".mvn/wrapper/maven-wrapper.jar";
+
+    /**
+     * Name of the property which should be used to override the default download url for the wrapper.
+     */
+    private static final String PROPERTY_NAME_WRAPPER_URL = "wrapperUrl";
+
+    public static void main(String args[]) {
+        System.out.println("- Downloader started");
+        File baseDirectory = new File(args[0]);
+        System.out.println("- Using base directory: " + baseDirectory.getAbsolutePath());
+
+        // If the maven-wrapper.properties exists, read it and check if it contains a custom
+        // wrapperUrl parameter.
+        File mavenWrapperPropertyFile = new File(baseDirectory, MAVEN_WRAPPER_PROPERTIES_PATH);
+        String url = DEFAULT_DOWNLOAD_URL;
+        if(mavenWrapperPropertyFile.exists()) {
+            FileInputStream mavenWrapperPropertyFileInputStream = null;
+            try {
+                mavenWrapperPropertyFileInputStream = new FileInputStream(mavenWrapperPropertyFile);
+                Properties mavenWrapperProperties = new Properties();
+                mavenWrapperProperties.load(mavenWrapperPropertyFileInputStream);
+                url = mavenWrapperProperties.getProperty(PROPERTY_NAME_WRAPPER_URL, url);
+            } catch (IOException e) {
+                System.out.println("- ERROR loading '" + MAVEN_WRAPPER_PROPERTIES_PATH + "'");
+            } finally {
+                try {
+                    if(mavenWrapperPropertyFileInputStream != null) {
+                        mavenWrapperPropertyFileInputStream.close();
+                    }
+                } catch (IOException e) {
+                    // Ignore ...
+                }
+            }
+        }
+        System.out.println("- Downloading from: : " + url);
+
+        File outputFile = new File(baseDirectory.getAbsolutePath(), MAVEN_WRAPPER_JAR_PATH);
+        if(!outputFile.getParentFile().exists()) {
+            if(!outputFile.getParentFile().mkdirs()) {
+                System.out.println(
+                        "- ERROR creating output direcrory '" + outputFile.getParentFile().getAbsolutePath() + "'");
+            }
+        }
+        System.out.println("- Downloading to: " + outputFile.getAbsolutePath());
+        try {
+            downloadFileFromURL(url, outputFile);
+            System.out.println("Done");
+            System.exit(0);
+        } catch (Throwable e) {
+            System.out.println("- Error downloading");
+            e.printStackTrace();
+            System.exit(1);
+        }
+    }
+
+    private static void downloadFileFromURL(String urlString, File destination) throws Exception {
+        URL website = new URL(urlString);
+        ReadableByteChannel rbc;
+        rbc = Channels.newChannel(website.openStream());
+        FileOutputStream fos = new FileOutputStream(destination);
+        fos.getChannel().transferFrom(rbc, 0, Long.MAX_VALUE);
+        fos.close();
+        rbc.close();
+    }
+
+}

Diff for: .mvn/wrapper/maven-wrapper.jar

@@ -1 +1 @@
-distributionUrl=https://repo1.maven.org/maven2/org/apache/maven/apache-maven/3.5.0/apache-maven-3.5.0-bin.zip
+distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.5.4/apache-maven-3.5.4-bin.zip

Diff for: .mvn/wrapper/maven-wrapper.properties

@@ -0,0 +1,72 @@
+# Contributing
+
+When contributing to this repository, please first discuss the change you wish to make via issue,
+email, or any other method with the owners of this repository before making a change. 
+
+Please note we have a code of conduct, please follow it in all your interactions with the project.
+
+## Code Standards
+
+#### General
+1. Do not use field injection (ie. `@Value`, `@Autowired`)
+    - Instead use an `@Autowired` or `@Value` annotated constructor
+    - Provide a static builder (ie. Lombok `@Builder` annotation)
+    - This helps to improves testability
+    - Helps to decouple from Spring
+    - If your constructor is feeling messy or too big - you are probably overloading the class you are working on
+2. Do not use any implementation specific JPA code (ie. Hibernate-only annotations)
+    - Exception for when no alternative functionality exists (ie. Postgres JSON field search)
+3. All of our code is auto-formatted to Google Java Format using the [fmt-maven-plugin](https://mvnrepository.com/artifact/com.coveo/fmt-maven-plugin) on build:
+```xml
+<plugin>
+    <groupId>com.coveo</groupId>
+    <artifactId>fmt-maven-plugin</artifactId>
+    <version>${FMT_MVN_PLG.VERSION}</version>
+    <executions>
+        <execution>
+            <goals>
+                <goal>format</goal>
+            </goals>
+        </execution>
+    </executions>
+</plugin>
+```
+5. Constants 
+- must be declared in a `@NoArgsConstructor(access=PRIVATE)` annotated class with a name representative of the type of constants. For example, the class `Tables` under the package `constants` would contain sql table names. 
+- Constant variable names should be consistent throughout code base. For example, the text `egoUserPermissions` should be defined by the variable `EGO_USER_PERMISSION`.  
+6. If a method is not stateful and not an interface/abstract method, then it should be static
+7. Never allow a method to return `null`. Instead, it should return `Optiona<T>` or an empty container type (something that has `.isEmpty()`)
+
+#### Service Layer
+1. Get * should always return Optional<T>
+2. Find * should always return a Collection<T>
+
+#### JPA
+1. Entity member declarations should take the following presidence:
+    1. @Id (identifier)    
+    2. Non-relationship @Column
+    3. @OneToOne
+    4. @OneToMany
+    5. @ManyToOne
+    6. @ManyToMany
+2. As explained in this [article](https://vladmihalcea.com/the-best-way-to-map-a-onetomany-association-with-jpa-and-hibernate/), you should prefer bidirectional associations since they are more efficient than unidirectional ones in terms of SQL performance [source](https://vladmihalcea.com/merge-entity-collections-jpa-hibernate/)
+3. Always lazy load for @OneToMany and @ManyToMany
+4. Never use CascadeType.ALL or CascadeType.REMOVE becuase they are too destructive. Use CascadeType.MERGE and CascadeType.PERSIST instead
+5. Name methods with `remove` indicating an entity was deleted
+6. Name methods with `dissociate` indicating a child relationship with its parent will be destoryed
+7. For performance reasons, @ManyToMany collections should be a Set as described [here](https://thoughts-on-java.org/association-mappings-bag-list-set/)
+8. For performance reasons, @OneToMany collections should be a list as described [here](https://vladmihalcea.com/hibernate-facts-favoring-sets-vs-bags/)
+9. In ManyToMany relationships, the JoinTable should only be defined on the **owning** side , and on the inverse side the `mappedBy` ManyToMany annotation parameter should be defined, as described [here](https://www.baeldung.com/hibernate-many-to-many)
+    
+### Testing
+1. Test FEATURES not methods
+2. Test method names should follow this convention: `[the name of the tested feature]_[expected input / tested state]_[expected behavior]`.
+
+#### General
+1. DB via Test Containers - no in-memory DB or OS specific services
+2. No dependencies on any external services (ie. production micro-service)
+3. Tests **DO NOT** clear their data between runs, meaning that no test should rely on or expect a clean DB when running
+
+##### Unit Testing
+
+##### Integration Testing

Diff for: CONTRIBUTING.md

@@ -27,7 +27,8 @@ RUN mkdir -p /srv/ego/install \
 
 # setup required environment variables
 ENV EGO_INSTALL_PATH /srv/ego
+ENV CONFIG_FILE /usr/src/app/src/main/resources/flyway/conf/flyway.conf
 
 # start ego server
 WORKDIR $EGO_INSTALL_PATH
-CMD $EGO_INSTALL_PATH/exec/run.sh
+CMD cd /usr/src/app;mvn "flyway:migrate" -Dflyway.configFiles=$CONFIG_FILE -Dflyway.password=password -Dflyway.url=jdbc:postgresql://postgres:5432/ego?stringtype=unspecified;$EGO_INSTALL_PATH/exec/run.sh

Diff for: Dockerfile

@@ -0,0 +1,16 @@
+FROM maven:3.6-jdk-8
+
+WORKDIR /usr/src/app
+
+ADD . .
+
+RUN mvn package -Dmaven.test.skip=true
+
+FROM java:8-alpine
+
+COPY --from=0 /usr/src/app/target/ego-*-SNAPSHOT-exec.jar /usr/bin/ego.jar
+COPY --from=0 /usr/src/app/src/main/resources/flyway/sql /usr/src/flyway-migration-sql
+
+ENTRYPOINT ["java", "-jar", "/usr/bin/ego.jar"]
+
+EXPOSE 8081/tcp

Diff for: Dockerfile.prod

@@ -0,0 +1,38 @@
+Notes
+------
+
+## 1. Reason for using the `@Type` hibernate annotation
+
+#### Problem
+In the entity `Policy`, the field `accessLevel` of type `AccessLevel` (which is an enum), the `@Type` annotation is used, which is a hibernate specific and not JPA annotation. The goal is to minimize or eliminate use of hibernate specific syntax, and just use JPA related code.
+
+#### Solutions
+The goal is to map the enum to the database. In the end, solution 3 was chosen.
+
+##### 1. Middleware Level Enum Handling without Hibernate Annotations
+Set the type of the field in the database to a `VARCHAR` and only use the `@Enumerated` JPA annotation
+Pros:
+    - Hibernate will handle the logic of converting an AccessLevel to a string. This means Enum conversion is handelled by the middleware naturally.
+    - Simple and clean solution using only JPA annotations
+Cons:
+    - Enum type is represented as an Enum at the application level but as a VARCHAR at the database level. If someone was to backdoor the database and update the `accessLevel` of a policy, they could potentially break the application. There is no safeguard outside of hibernate/JPA
+
+##### 2. Application Level Enum Handling
+Set the type of the field in the postgres database to a `AccessLevelType` and in the Java DAO, represent the field as a `String`. The application level (i.e the service layers) will manage the conversion of the Policies `String` accessLevel field to the `AccessLevel` Java enum type. Hibernate will pass the string to the postgres database, and since the url ends with `?stringtype=unspecified`, postgres will cast the string to the Database enum type
+Pros:
+    - No need for Hibernate annotations
+    - Since conversions are done manually at application layer, functionality is more obvious and cleaner
+Cons:
+    - Its manual, meaning, if the developer forgets to check that the conversion from `AccessLevel->String` and `String->AccessLevel` is correct, a potentially wrong string value will be passed from hibernate to postrgres, resulting in a postgres error
+
+
+##### 3. Middleware Level Enum Handling WITH Hibernate Annotations
+Follow the instructions from this [blog post](https://vladmihalcea.com/the-best-way-to-map-an-enum-type-with-jpa-and-hiberate/) under the heading `Mapping a Java Enum to a database-specific Enumarated column type`. This results in the use of the `@Type` hibernate specific annotation for the `Policy` entity. This annotation modifies the way hibernate process the `accessLevel` field.
+Pros:
+    - All processing is done at the middleware (hibernate) and the developer does not need to add any extra code at the application layer. 
+    - Hibernate properly process the Java enum type to a Postgres enum type. This means **BOTH** the java code (the `Policy` entity) and the Policy table in postgres are types and protected from values outside the enumeration. 
+    - Almost no developer effort and minimizes developer mistakes
+Cons:
+    - The `Policy` entity is using a hibernate annotation with a custom `PostgresSQLEnumType` processor to assist hibernate in supporting Postgres enum types. 
+
+

Diff for: EgoDatabaseDiagram.pdf

@@ -8,6 +8,7 @@
   <a href="http://www.overture.bio/products/ego" target="_blank"><img alt="General Availability" title="General Availability" src="http://www.overture.bio/img/progress-horizontal-GA.svg" width="320" /></a>
 </p>
 
+[![BrowserStack Status](https://www.browserstack.com/automate/badge.svg?badge_key=U3dLZnRFNWI2MWNFY2NGcXVtVTB3WDcyU2dPVjlVeEFYUEdxUnpYZlhrUT0tLTFzY0taYTA0MVFEa3ErNkRZdTBRWVE9PQ==--690f89a41a0eedf7b4975bd7df2eac162e04e775% )](https://www.browserstack.com/automate/public-build/U3dLZnRFNWI2MWNFY2NGcXVtVTB3WDcyU2dPVjlVeEFYUEdxUnpYZlhrUT0tLTFzY0taYTA0MVFEa3ErNkRZdTBRWVE9PQ==--690f89a41a0eedf7b4975bd7df2eac162e04e775%)
 [![Build Status](https://travis-ci.org/overture-stack/ego.svg?branch=master)](https://travis-ci.org/overture-stack/ego)
 [![CircleCI](https://circleci.com/gh/overture-stack/ego/tree/develop.svg?style=svg)](https://circleci.com/gh/overture-stack/ego/tree/develop)
 [![Slack](http://slack.overture.bio/badge.svg)](http://slack.overture.bio)
@@ -23,6 +24,8 @@
   - [Step 2 - Run](#step-2---run)
 - [Tech Specifications](#tech-specification)
 - [Usage](#usage)
+- [Shoutouts](#shoutouts)
+  - [Browserstack](#browserstack)
 
 ## Introduction
 
@@ -91,13 +94,13 @@ Database migrations and versioning is managed by [flyway](https://flywaydb.org/)
 Get current version information:
 
 ```bash
-./flyway -configFiles=<path_to_ego>/ego/src/main/resources/flyway/conf/flyway.conf -locations=filesystem:<path_to_ego>/ego/src/main/resources/flyway/sql info
+./fly
 ```
 
 Run outstanding migrations:
 
 ```bash
-./flyway -configFiles=<path_to_ego>/ego/src/main/resources/flyway/conf/flyway.conf -locations=filesystem:<path_to_ego>/ego/src/main/resources/flyway/sql migrate
+./fly migrate
 ```
 
 To see the migration naming convention, [click here.](https://flywaydb.org/documentation/migrations#naming)
@@ -163,7 +166,6 @@ An example ego JWT is mentioned below:
 ```
 
 #### Notes
-
 - "aud" field can contain one or more client IDs. This field indicates the client services that are authorized to use this JWT.
 - "groups" will differ based on the domain of client services - each domain of service should get list of groups from that domain's ego service.
 - "permissions" will differ based on domain of client service - each domain of service should get list of permissions from that domain's ego service.
@@ -204,3 +206,11 @@ curl example:
   -H 'Content-Type: application/x-www-form-urlencoded' \
   -d 'grant_type=client_credentials&client_id=my-app-id&client_secret=secretpassword'
 ```
+
+## Shoutouts
+
+### Browserstack
+Many thanks to [Browserstack](https://www.browserstack.com/) for giving our test capabilities a powerup!
+![Browserstack](https://p14.zdusercontent.com/attachment/1015988/qyPFNKIZXCbr4qKjd5oxrayZc?token=eyJhbGciOiJkaXIiLCJlbmMiOiJBMTI4Q0JDLUhTMjU2In0..-yKqMwgZKdCDJZmW2kcVYw.IKGbK6GBbU3uZ3B7Vapw8uZeQ-uhXDV9kANtz5OOOBl0Ceg6Oi1gS5wqBnStOsCKgb3CibgGIrYjk-odWPwaL9Ei0u3OIDuBldkxF6aJ6eGtC9G4LfbDLGtOnYkUiANvx5HNPb7HZa3QyivKxCcX_MjO5U01F0WbmJajfYBsFVHHLtO0dBqFz-eWZMmLB0yfjZEaVPAUfLk9H1TO4c6Vw91Or29FrzaoGDQmvmcP7Pg00LMoxuaLxGJuuOiUlEe6OunidzxRd_elUZxMJ_caonvHEjSCkq_yHilG67tGewY.IV6Qg9p5vE0TGk59pqZtRg)
+
+