Repository: urbandataanalytics/SwissKnife

Issue:
GCloudStorage.list_blobs don't use BUCKET_PREFIX_PATH
**Describe the bug**
Currently, GCloudStorage.list_blobs function don't use BUCKET_PREFIX_PATH. So, the result of "list_blobs" will be incorrect.

**To Reproduce**
Steps to reproduce the behavior:
1. For example, export the variable *BUCKET_PATH=gs://example-bucket/prefix-path*
2. Execute list_blobs on *storage_path* "path-to-list".
5. It will list "gs://example-bucket/path-to-list"

**Expected behavior**
Using the parameters of the previous example, the result must have the listing of the path "gs://example-bucket/prefix-path/path-to-list".

**Desktop (please complete the following information):**
 - OS: Ubuntu

**Python version:**
Python 3.6.9


Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index bcb47d7..a07de52 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,7 +4,14 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [0.6.1] - 2020-05-28
+## [0.6.3] - 2020-05-29
+### Changed
+- Improved `GCloudStorage.get_storage_complete_file_path` using `os.path.join`.
+- `file_name` is optional in `GCloudStorage.get_storage_complete_file_path`.
+### Fixed
+- Now `GCloudStorage.list_blobs` can use the BUCKET_PREFIX_PATH. It has an optional param "with_prefix" (True by default). <issue #11>
+
+## [0.6.2] - 2020-05-28
 #### Added
 - Added a calendar function to get weeks numbers with format `2020W02`
 
@@ -85,4 +92,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [0.1.0] - 2019-10-31
 ### Added
 - `ExecutionEnvironment` object.
-- `CURRENT_ENVIRONMENT` variable (located in `SwissKnife.info.CURRENT_ENVIRONMENT`) that return a ExecutionEnvironment with the current environment setted.
\ No newline at end of file
+- `CURRENT_ENVIRONMENT` variable (located in `SwissKnife.info.CURRENT_ENVIRONMENT`) that return a ExecutionEnvironment with the current environment setted.
diff --git a/SwissKnife/gcloud/GCloudStorage.py b/SwissKnife/gcloud/GCloudStorage.py
index 765e715..b724479 100644
--- a/SwissKnife/gcloud/GCloudStorage.py
+++ b/SwissKnife/gcloud/GCloudStorage.py
@@ -1,4 +1,5 @@
 import sys
+import os
 import logging
 import google.cloud.storage as gcloud
 
@@ -138,7 +139,7 @@ class GCloudStorage:
         return blob
     
     @staticmethod
-    def get_storage_complete_file_path(file_name: str,
+    def get_storage_complete_file_path(file_name: str=None,
                                        file_path: str=None,
                                        with_bucket: bool=False,
                                        with_prefix: bool=True,
@@ -158,21 +159,26 @@ class GCloudStorage:
         :return: Complete path of a file stored in gcloud
         :rtype: str
         """
-        bucket_name = f'{BUCKET_NAME}/' if with_bucket else ''
-        file_prefix = f'{BUCKET_PATH_PREFIX}/' if with_prefix and BUCKET_PATH_PREFIX else ''
-        path = f'{file_path}/' if file_path else ''
+        bucket_name = BUCKET_NAME if with_bucket else ''
+        file_prefix = BUCKET_PATH_PREFIX if with_prefix and BUCKET_PATH_PREFIX else ''
+        final_file_path = file_path if file_path else ''
         gs_prefix = 'gs://' if with_gs else ''
+        final_file_name = file_name if file_name else ''
         
-        return f'{gs_prefix}{bucket_name}{file_prefix}{path}{file_name}'
+        return os.path.join(gs_prefix, bucket_name, file_prefix, final_file_path, final_file_name)
 
-    def list_blobs(self, storage_path: str) -> "Iterator":
+    def list_blobs(self, storage_path: str, with_prefix : bool = True) -> "Iterator":
         """Lists all the files that exists in the specified path.
         This is similar to GNU's `ls` command. Returns an iterator
         that should be treated later on.
 
-        :param storage_path: Parent path from which files will be listed
+        :param storage_path: Parent path from which files will be listed(without slash at the end)
         :type storage_path: str
+        :param with_prefix: this adds or not the BUCKET_PREFIX_PATH to the resulting path
+        :type with_prefix: bool
         :return: Iterator with blobs contained in the parent path
         :rtype: "Iterator"
         """
-        return self.bucket.list_blobs(prefix=storage_path)
\ No newline at end of file
+
+        list_prefix = GCloudStorage.get_storage_complete_file_path(file_path=storage_path, with_prefix=with_prefix, with_gs=False)
+        return self.bucket.list_blobs(prefix=list_prefix)
\ No newline at end of file


Test patch:
diff --git a/tests/gcloud/test_GCloudStorage.py b/tests/gcloud/test_GCloudStorage.py
index 7eabf53..ceedfa1 100644
--- a/tests/gcloud/test_GCloudStorage.py
+++ b/tests/gcloud/test_GCloudStorage.py
@@ -3,6 +3,7 @@ import imp
 import unittest
 import SwissKnife
 import tests.test_utils as test_utils
+from unittest.mock import MagicMock
 
 from unittest import mock
 from SwissKnife.gcloud.GCloudStorage import GCloudStorage
@@ -47,6 +48,17 @@ class test_GCloudStorage(unittest.TestCase):
                                                               with_prefix=False)
         
         self.assertEqual(expected, result)
+
+    def test_path_without_file_name(self):
+        file_path = 'random/path'
+        expected = f'gs://fancy-bucket/{file_path}/'
+                
+        result = GCloudStorage.get_storage_complete_file_path(
+                                                              file_path=file_path,
+                                                              with_bucket=True,
+                                                              with_prefix=False
+                                                             )
+        self.assertEqual(expected, result)
     
     @mock.patch('SwissKnife.gcloud.GCloudStorage.gcloud')
     def test_correct_data_type_to_upload(self, mock_gcloud):
@@ -62,8 +74,7 @@ class test_GCloudStorage(unittest.TestCase):
                                                           dt,
                                                           file_path,
                                                           file_name))
-        
-        
+
     @mock.patch('SwissKnife.gcloud.GCloudStorage.gcloud')
     def test_incorrect_data_type_to_upload(self, mock_gcloud):
         gc = GCloudStorage()
@@ -79,9 +90,51 @@ class test_GCloudStorage(unittest.TestCase):
                                     dt,
                                     file_path,
                                     file_name)
-    
+
+    @mock.patch('SwissKnife.gcloud.GCloudStorage.gcloud')
+    def test_valid_path_in_list_blobs_with_prefix(self, mock_gcloud):
+
+
+        storage_path = 'random_path'
+        expected = f'{self.bucket_path_prefix}/{storage_path}/'
+
+        def assert_path_in_list_blobs(prefix):
+
+            self.assertEqual(prefix, expected)
+
+        mocked_bucket = MagicMock()
+        mocked_bucket.list_blobs = assert_path_in_list_blobs
+        
+        gc = GCloudStorage()
+        gc.bucket = mocked_bucket
+
+        gc.list_blobs(storage_path)
+
+    @mock.patch('SwissKnife.gcloud.GCloudStorage.gcloud')
+    def test_valid_path_in_list_blobs_without_prefix(self, mock_gcloud):
+
+
+        storage_path = 'random_path'
+        expected = f'{storage_path}/'
+
+        def assert_path_in_list_blobs(prefix):
+
+            self.assertEqual(prefix, expected)
+
+        mocked_bucket = MagicMock()
+        mocked_bucket.list_blobs = assert_path_in_list_blobs
+        
+        gc = GCloudStorage()
+        gc.bucket = mocked_bucket
+
+        gc.list_blobs(storage_path, with_prefix=False)
+
+
+
+
     """
         The rest of methods are not tested in this class, as they are mere
         rewrittings of code provided by Google. This tests would require, 
         as well, to have a working internet connection and a configured SA.
-    """
\ No newline at end of file
+    """
+