galovics commented on code in PR #2915:
URL: https://github.com/apache/fineract/pull/2915#discussion_r1085234373


##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/api/RunreportsApiResource.java:
##########
@@ -70,6 +72,29 @@ public RunreportsApiResource(final PlatformSecurityContext 
context, final ReadRe
         this.reportingProcessServiceProvider = reportingProcessServiceProvider;
     }
 
+    @GET
+    @Path("/availableExports/{reportName}")
+    @Consumes({ MediaType.APPLICATION_JSON })
+    @Produces({ MediaType.APPLICATION_JSON })
+    @Operation(summary = "Return all available export types for the specific 
report", description = "Returns the list of all available export types.")
+    @ApiResponses({
+            @ApiResponse(responseCode = "200", description = "", content = 
@Content(array = @ArraySchema(schema = @Schema(implementation = 
ReportExportType.class)))) })
+    public Response retrieveAllAvailableExports(@PathParam("reportName") 
@Parameter(description = "reportName") final String reportName,
+            @Context final UriInfo uriInfo,
+            @DefaultValue("false") 
@QueryParam(IS_SELF_SERVICE_USER_REPORT_PARAMETER) @Parameter(description = 
IS_SELF_SERVICE_USER_REPORT_PARAMETER) final boolean isSelfServiceUserReport) {
+        MultivaluedMap<String, String> queryParams = new 
MultivaluedStringMap();
+        queryParams.putAll(uriInfo.getQueryParameters());
+
+        final boolean parameterType = 
ApiParameterHelper.parameterType(queryParams);
+        String reportType = 
this.readExtraDataAndReportingService.getReportType(reportName, 
isSelfServiceUserReport, parameterType);

Review Comment:
   `this.` not needed



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/data/ReportExportType.java:
##########
@@ -0,0 +1,34 @@
+/**
+ * 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.fineract.infrastructure.dataqueries.data;
+
+import java.io.Serializable;
+import lombok.Data;
+
+@Data
+public class ReportExportType implements Serializable {

Review Comment:
   `@RequiredArgsConstructor`



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/api/RunreportsApiResource.java:
##########
@@ -70,6 +72,29 @@ public RunreportsApiResource(final PlatformSecurityContext 
context, final ReadRe
         this.reportingProcessServiceProvider = reportingProcessServiceProvider;
     }
 
+    @GET
+    @Path("/availableExports/{reportName}")
+    @Consumes({ MediaType.APPLICATION_JSON })
+    @Produces({ MediaType.APPLICATION_JSON })
+    @Operation(summary = "Return all available export types for the specific 
report", description = "Returns the list of all available export types.")
+    @ApiResponses({
+            @ApiResponse(responseCode = "200", description = "", content = 
@Content(array = @ArraySchema(schema = @Schema(implementation = 
ReportExportType.class)))) })
+    public Response retrieveAllAvailableExports(@PathParam("reportName") 
@Parameter(description = "reportName") final String reportName,
+            @Context final UriInfo uriInfo,
+            @DefaultValue("false") 
@QueryParam(IS_SELF_SERVICE_USER_REPORT_PARAMETER) @Parameter(description = 
IS_SELF_SERVICE_USER_REPORT_PARAMETER) final boolean isSelfServiceUserReport) {
+        MultivaluedMap<String, String> queryParams = new 
MultivaluedStringMap();
+        queryParams.putAll(uriInfo.getQueryParameters());
+
+        final boolean parameterType = 
ApiParameterHelper.parameterType(queryParams);
+        String reportType = 
this.readExtraDataAndReportingService.getReportType(reportName, 
isSelfServiceUserReport, parameterType);
+        ReportingProcessService reportingProcessService = 
this.reportingProcessServiceProvider.findReportingProcessService(reportType);
+        if (reportingProcessService == null) {

Review Comment:
   Let's not put this logic here but in the provider.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableReportingProcessService.java:
##########
@@ -18,57 +18,79 @@
  */
 package org.apache.fineract.infrastructure.dataqueries.service;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.io.IOException;
+import java.text.SimpleDateFormat;
+import java.util.ArrayList;
+import java.util.Date;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.MultivaluedMap;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.Response.ResponseBuilder;
 import javax.ws.rs.core.StreamingOutput;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.lang3.StringUtils;
+import 
org.apache.fineract.infrastructure.configuration.domain.ConfigurationDomainService;
 import org.apache.fineract.infrastructure.core.api.ApiParameterHelper;
 import 
org.apache.fineract.infrastructure.core.serialization.ToApiJsonSerializer;
 import 
org.apache.fineract.infrastructure.dataqueries.api.RunreportsApiResource;
 import 
org.apache.fineract.infrastructure.dataqueries.data.GenericResultsetData;
 import org.apache.fineract.infrastructure.dataqueries.data.ReportData;
+import org.apache.fineract.infrastructure.dataqueries.data.ReportExportType;
 import org.apache.fineract.infrastructure.report.annotation.ReportService;
 import 
org.apache.fineract.infrastructure.report.service.ReportingProcessService;
-import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
+import 
software.amazon.awssdk.auth.credentials.EnvironmentVariableCredentialsProvider;
+import software.amazon.awssdk.core.SdkSystemSetting;
+import software.amazon.awssdk.core.sync.RequestBody;
+import software.amazon.awssdk.regions.Region;
+import software.amazon.awssdk.services.s3.S3Client;
 
 @Service
 @ReportService(type = { "Table", "Chart", "SMS" })
+@RequiredArgsConstructor
+@Slf4j
 public class DatatableReportingProcessService implements 
ReportingProcessService {
 
+    public static final String AWS_S_3_BUCKET_NAME = "AWS_S3_BUCKET_NAME";

Review Comment:
   This doesn't belong here, let's use the FineractProperties for this.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableReportingProcessService.java:
##########
@@ -90,13 +112,64 @@ private Response exportJSON(String reportName, 
MultivaluedMap<String, String> qu
         return 
Response.ok().entity(json).type(MediaType.APPLICATION_JSON).build();
     }
 
-    private Response exportS3(String reportName, MultivaluedMap<String, 
String> queryParams, Map<String, String> reportParams,
-            boolean isSelfServiceUserReport, String parameterTypeValue) {
-        throw new UnsupportedOperationException("S3 export not supported for 
datatables");
+    private boolean isAwsCredentialsValid() {

Review Comment:
   This definitely doesn't belong here. Why can't we use the AWS SDK provided 
`AwsCredentialsProvider`?



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableReportingProcessService.java:
##########
@@ -90,13 +112,64 @@ private Response exportJSON(String reportName, 
MultivaluedMap<String, String> qu
         return 
Response.ok().entity(json).type(MediaType.APPLICATION_JSON).build();
     }
 
-    private Response exportS3(String reportName, MultivaluedMap<String, 
String> queryParams, Map<String, String> reportParams,
-            boolean isSelfServiceUserReport, String parameterTypeValue) {
-        throw new UnsupportedOperationException("S3 export not supported for 
datatables");
+    private boolean isAwsCredentialsValid() {
+        return 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_ACCESS_KEY_ID.environmentVariable()))
+                && 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_SECRET_ACCESS_KEY.environmentVariable()))
+                && 
StringUtils.isNotBlank(SdkSystemSetting.AWS_REGION.environmentVariable());
+    }
+
+    private Response exportS3(String reportName, Map<String, String> 
reportParams, boolean isSelfServiceUserReport,
+            String parameterTypeValue) {
+        if (!isAwsCredentialsValid()) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS 
credentials not set").build();
+        }
+        if (StringUtils.isBlank(System.getenv().get(AWS_S_3_BUCKET_NAME))) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS S3 
bucket name not set").build();
+        }
+        try {
+            S3Client s3Client = 
S3Client.builder().region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())))
+                    
.credentialsProvider(EnvironmentVariableCredentialsProvider.create()).build();
+            StreamingOutput output = 
this.readExtraDataAndReportingService.retrieveReportCSV(reportName, 
parameterTypeValue, reportParams,

Review Comment:
   `this.` not needed



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableReportingProcessService.java:
##########
@@ -90,13 +112,64 @@ private Response exportJSON(String reportName, 
MultivaluedMap<String, String> qu
         return 
Response.ok().entity(json).type(MediaType.APPLICATION_JSON).build();
     }
 
-    private Response exportS3(String reportName, MultivaluedMap<String, 
String> queryParams, Map<String, String> reportParams,
-            boolean isSelfServiceUserReport, String parameterTypeValue) {
-        throw new UnsupportedOperationException("S3 export not supported for 
datatables");
+    private boolean isAwsCredentialsValid() {
+        return 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_ACCESS_KEY_ID.environmentVariable()))
+                && 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_SECRET_ACCESS_KEY.environmentVariable()))
+                && 
StringUtils.isNotBlank(SdkSystemSetting.AWS_REGION.environmentVariable());
+    }
+
+    private Response exportS3(String reportName, Map<String, String> 
reportParams, boolean isSelfServiceUserReport,
+            String parameterTypeValue) {
+        if (!isAwsCredentialsValid()) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS 
credentials not set").build();
+        }
+        if (StringUtils.isBlank(System.getenv().get(AWS_S_3_BUCKET_NAME))) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS S3 
bucket name not set").build();
+        }
+        try {
+            S3Client s3Client = 
S3Client.builder().region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())))

Review Comment:
   Can't we create the S3Client upon startup?



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableReportingProcessService.java:
##########
@@ -90,13 +112,64 @@ private Response exportJSON(String reportName, 
MultivaluedMap<String, String> qu
         return 
Response.ok().entity(json).type(MediaType.APPLICATION_JSON).build();
     }
 
-    private Response exportS3(String reportName, MultivaluedMap<String, 
String> queryParams, Map<String, String> reportParams,
-            boolean isSelfServiceUserReport, String parameterTypeValue) {
-        throw new UnsupportedOperationException("S3 export not supported for 
datatables");
+    private boolean isAwsCredentialsValid() {
+        return 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_ACCESS_KEY_ID.environmentVariable()))
+                && 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_SECRET_ACCESS_KEY.environmentVariable()))
+                && 
StringUtils.isNotBlank(SdkSystemSetting.AWS_REGION.environmentVariable());
+    }
+
+    private Response exportS3(String reportName, Map<String, String> 
reportParams, boolean isSelfServiceUserReport,
+            String parameterTypeValue) {
+        if (!isAwsCredentialsValid()) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS 
credentials not set").build();
+        }
+        if (StringUtils.isBlank(System.getenv().get(AWS_S_3_BUCKET_NAME))) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS S3 
bucket name not set").build();
+        }
+        try {
+            S3Client s3Client = 
S3Client.builder().region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())))
+                    
.credentialsProvider(EnvironmentVariableCredentialsProvider.create()).build();
+            StreamingOutput output = 
this.readExtraDataAndReportingService.retrieveReportCSV(reportName, 
parameterTypeValue, reportParams,

Review Comment:
   It's not your fault, just noting but I really really don't like this 
StreamingOutput interface here, it's a javax.ws.rs class which is not supposed 
to be on this level but on the API.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableReportingProcessService.java:
##########
@@ -90,13 +112,64 @@ private Response exportJSON(String reportName, 
MultivaluedMap<String, String> qu
         return 
Response.ok().entity(json).type(MediaType.APPLICATION_JSON).build();
     }
 
-    private Response exportS3(String reportName, MultivaluedMap<String, 
String> queryParams, Map<String, String> reportParams,
-            boolean isSelfServiceUserReport, String parameterTypeValue) {
-        throw new UnsupportedOperationException("S3 export not supported for 
datatables");
+    private boolean isAwsCredentialsValid() {
+        return 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_ACCESS_KEY_ID.environmentVariable()))
+                && 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_SECRET_ACCESS_KEY.environmentVariable()))
+                && 
StringUtils.isNotBlank(SdkSystemSetting.AWS_REGION.environmentVariable());
+    }
+
+    private Response exportS3(String reportName, Map<String, String> 
reportParams, boolean isSelfServiceUserReport,
+            String parameterTypeValue) {
+        if (!isAwsCredentialsValid()) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS 
credentials not set").build();
+        }
+        if (StringUtils.isBlank(System.getenv().get(AWS_S_3_BUCKET_NAME))) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS S3 
bucket name not set").build();
+        }
+        try {
+            S3Client s3Client = 
S3Client.builder().region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())))
+                    
.credentialsProvider(EnvironmentVariableCredentialsProvider.create()).build();
+            StreamingOutput output = 
this.readExtraDataAndReportingService.retrieveReportCSV(reportName, 
parameterTypeValue, reportParams,
+                    isSelfServiceUserReport);
+            ByteArrayOutputStream byteArrayOutputStream = new 
ByteArrayOutputStream();

Review Comment:
   try with resources?



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableReportingProcessService.java:
##########
@@ -90,13 +112,64 @@ private Response exportJSON(String reportName, 
MultivaluedMap<String, String> qu
         return 
Response.ok().entity(json).type(MediaType.APPLICATION_JSON).build();
     }
 
-    private Response exportS3(String reportName, MultivaluedMap<String, 
String> queryParams, Map<String, String> reportParams,
-            boolean isSelfServiceUserReport, String parameterTypeValue) {
-        throw new UnsupportedOperationException("S3 export not supported for 
datatables");
+    private boolean isAwsCredentialsValid() {
+        return 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_ACCESS_KEY_ID.environmentVariable()))
+                && 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_SECRET_ACCESS_KEY.environmentVariable()))
+                && 
StringUtils.isNotBlank(SdkSystemSetting.AWS_REGION.environmentVariable());
+    }
+
+    private Response exportS3(String reportName, Map<String, String> 
reportParams, boolean isSelfServiceUserReport,
+            String parameterTypeValue) {
+        if (!isAwsCredentialsValid()) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS 
credentials not set").build();
+        }
+        if (StringUtils.isBlank(System.getenv().get(AWS_S_3_BUCKET_NAME))) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS S3 
bucket name not set").build();
+        }
+        try {
+            S3Client s3Client = 
S3Client.builder().region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())))
+                    
.credentialsProvider(EnvironmentVariableCredentialsProvider.create()).build();
+            StreamingOutput output = 
this.readExtraDataAndReportingService.retrieveReportCSV(reportName, 
parameterTypeValue, reportParams,
+                    isSelfServiceUserReport);
+            ByteArrayOutputStream byteArrayOutputStream = new 
ByteArrayOutputStream();
+            output.write(byteArrayOutputStream);
+            byteArrayOutputStream.close();
+            String finalS3Path = getS3FolderName() + "" + 
exportFileNameGeneration(reportName, reportParams) + ".csv";

Review Comment:
   Just a very minor thing but can we construct this path value with 
String.format? Also, is it intentional to have an empty String in the middle?
   
   Bonus: would be awesome if you'd extract this to a factory class so that you 
can separately write unit test for it.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableReportingProcessService.java:
##########
@@ -90,13 +112,64 @@ private Response exportJSON(String reportName, 
MultivaluedMap<String, String> qu
         return 
Response.ok().entity(json).type(MediaType.APPLICATION_JSON).build();
     }
 
-    private Response exportS3(String reportName, MultivaluedMap<String, 
String> queryParams, Map<String, String> reportParams,
-            boolean isSelfServiceUserReport, String parameterTypeValue) {
-        throw new UnsupportedOperationException("S3 export not supported for 
datatables");
+    private boolean isAwsCredentialsValid() {
+        return 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_ACCESS_KEY_ID.environmentVariable()))
+                && 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_SECRET_ACCESS_KEY.environmentVariable()))
+                && 
StringUtils.isNotBlank(SdkSystemSetting.AWS_REGION.environmentVariable());
+    }
+
+    private Response exportS3(String reportName, Map<String, String> 
reportParams, boolean isSelfServiceUserReport,
+            String parameterTypeValue) {
+        if (!isAwsCredentialsValid()) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS 
credentials not set").build();
+        }
+        if (StringUtils.isBlank(System.getenv().get(AWS_S_3_BUCKET_NAME))) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS S3 
bucket name not set").build();
+        }
+        try {
+            S3Client s3Client = 
S3Client.builder().region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())))
+                    
.credentialsProvider(EnvironmentVariableCredentialsProvider.create()).build();
+            StreamingOutput output = 
this.readExtraDataAndReportingService.retrieveReportCSV(reportName, 
parameterTypeValue, reportParams,
+                    isSelfServiceUserReport);
+            ByteArrayOutputStream byteArrayOutputStream = new 
ByteArrayOutputStream();
+            output.write(byteArrayOutputStream);
+            byteArrayOutputStream.close();
+            String finalS3Path = getS3FolderName() + "" + 
exportFileNameGeneration(reportName, reportParams) + ".csv";
+            s3Client.putObject(builder -> 
builder.bucket(System.getenv().get(AWS_S_3_BUCKET_NAME)).key(finalS3Path).build(),
+                    
RequestBody.fromBytes(byteArrayOutputStream.toByteArray()));
+            return Response.noContent().build();
+        } catch (IOException e) {
+            log.error("Error while exporting to S3", e);

Review Comment:
   No need for the log message if you're rethrowing the exception below.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableReportingProcessService.java:
##########
@@ -90,13 +112,64 @@ private Response exportJSON(String reportName, 
MultivaluedMap<String, String> qu
         return 
Response.ok().entity(json).type(MediaType.APPLICATION_JSON).build();
     }
 
-    private Response exportS3(String reportName, MultivaluedMap<String, 
String> queryParams, Map<String, String> reportParams,
-            boolean isSelfServiceUserReport, String parameterTypeValue) {
-        throw new UnsupportedOperationException("S3 export not supported for 
datatables");
+    private boolean isAwsCredentialsValid() {
+        return 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_ACCESS_KEY_ID.environmentVariable()))
+                && 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_SECRET_ACCESS_KEY.environmentVariable()))
+                && 
StringUtils.isNotBlank(SdkSystemSetting.AWS_REGION.environmentVariable());
+    }
+
+    private Response exportS3(String reportName, Map<String, String> 
reportParams, boolean isSelfServiceUserReport,
+            String parameterTypeValue) {
+        if (!isAwsCredentialsValid()) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS 
credentials not set").build();
+        }
+        if (StringUtils.isBlank(System.getenv().get(AWS_S_3_BUCKET_NAME))) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS S3 
bucket name not set").build();
+        }
+        try {
+            S3Client s3Client = 
S3Client.builder().region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())))
+                    
.credentialsProvider(EnvironmentVariableCredentialsProvider.create()).build();
+            StreamingOutput output = 
this.readExtraDataAndReportingService.retrieveReportCSV(reportName, 
parameterTypeValue, reportParams,
+                    isSelfServiceUserReport);
+            ByteArrayOutputStream byteArrayOutputStream = new 
ByteArrayOutputStream();
+            output.write(byteArrayOutputStream);
+            byteArrayOutputStream.close();
+            String finalS3Path = getS3FolderName() + "" + 
exportFileNameGeneration(reportName, reportParams) + ".csv";
+            s3Client.putObject(builder -> 
builder.bucket(System.getenv().get(AWS_S_3_BUCKET_NAME)).key(finalS3Path).build(),
+                    
RequestBody.fromBytes(byteArrayOutputStream.toByteArray()));
+            return Response.noContent().build();
+        } catch (IOException e) {
+            log.error("Error while exporting to S3", e);
+            throw new IllegalStateException("Error while exporting to S3!", e);

Review Comment:
   Exclamation mark not necessary, is it? :)



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableReportingProcessService.java:
##########
@@ -90,13 +112,64 @@ private Response exportJSON(String reportName, 
MultivaluedMap<String, String> qu
         return 
Response.ok().entity(json).type(MediaType.APPLICATION_JSON).build();
     }
 
-    private Response exportS3(String reportName, MultivaluedMap<String, 
String> queryParams, Map<String, String> reportParams,
-            boolean isSelfServiceUserReport, String parameterTypeValue) {
-        throw new UnsupportedOperationException("S3 export not supported for 
datatables");
+    private boolean isAwsCredentialsValid() {
+        return 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_ACCESS_KEY_ID.environmentVariable()))
+                && 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_SECRET_ACCESS_KEY.environmentVariable()))
+                && 
StringUtils.isNotBlank(SdkSystemSetting.AWS_REGION.environmentVariable());
+    }
+
+    private Response exportS3(String reportName, Map<String, String> 
reportParams, boolean isSelfServiceUserReport,
+            String parameterTypeValue) {
+        if (!isAwsCredentialsValid()) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS 
credentials not set").build();
+        }
+        if (StringUtils.isBlank(System.getenv().get(AWS_S_3_BUCKET_NAME))) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS S3 
bucket name not set").build();
+        }
+        try {
+            S3Client s3Client = 
S3Client.builder().region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())))
+                    
.credentialsProvider(EnvironmentVariableCredentialsProvider.create()).build();

Review Comment:
   Let's not hardcode the EnvironmentVariableCredentialsProvider, we need the 
`DefaultCredentialsProvider` so that it'll read from multiple places.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableReportingProcessService.java:
##########
@@ -90,13 +112,64 @@ private Response exportJSON(String reportName, 
MultivaluedMap<String, String> qu
         return 
Response.ok().entity(json).type(MediaType.APPLICATION_JSON).build();
     }
 
-    private Response exportS3(String reportName, MultivaluedMap<String, 
String> queryParams, Map<String, String> reportParams,
-            boolean isSelfServiceUserReport, String parameterTypeValue) {
-        throw new UnsupportedOperationException("S3 export not supported for 
datatables");
+    private boolean isAwsCredentialsValid() {
+        return 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_ACCESS_KEY_ID.environmentVariable()))
+                && 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_SECRET_ACCESS_KEY.environmentVariable()))
+                && 
StringUtils.isNotBlank(SdkSystemSetting.AWS_REGION.environmentVariable());
+    }
+
+    private Response exportS3(String reportName, Map<String, String> 
reportParams, boolean isSelfServiceUserReport,
+            String parameterTypeValue) {
+        if (!isAwsCredentialsValid()) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS 
credentials not set").build();
+        }
+        if (StringUtils.isBlank(System.getenv().get(AWS_S_3_BUCKET_NAME))) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS S3 
bucket name not set").build();
+        }
+        try {
+            S3Client s3Client = 
S3Client.builder().region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())))
+                    
.credentialsProvider(EnvironmentVariableCredentialsProvider.create()).build();
+            StreamingOutput output = 
this.readExtraDataAndReportingService.retrieveReportCSV(reportName, 
parameterTypeValue, reportParams,
+                    isSelfServiceUserReport);
+            ByteArrayOutputStream byteArrayOutputStream = new 
ByteArrayOutputStream();
+            output.write(byteArrayOutputStream);
+            byteArrayOutputStream.close();
+            String finalS3Path = getS3FolderName() + "" + 
exportFileNameGeneration(reportName, reportParams) + ".csv";
+            s3Client.putObject(builder -> 
builder.bucket(System.getenv().get(AWS_S_3_BUCKET_NAME)).key(finalS3Path).build(),
+                    
RequestBody.fromBytes(byteArrayOutputStream.toByteArray()));
+            return Response.noContent().build();
+        } catch (IOException e) {
+            log.error("Error while exporting to S3", e);
+            throw new IllegalStateException("Error while exporting to S3!", e);
+        }
+    }
+
+    private String getS3FolderName() {
+        String folderName = 
configurationDomainService.retrieveReportExportS3FolderName();
+        if (StringUtils.isNotBlank(folderName)) {
+            folderName = folderName + "/";
+        }
+        return folderName.trim();
+    }
+
+    private String exportFileNameGeneration(String reportName, Map<String, 
String> reportParams) {
+        String timestamp = "_" + new 
SimpleDateFormat("yyyyMMddHHmmss").format(new Date());

Review Comment:
   Careful, this is the old Date API on one hand, it's better to use the new 
java.time package for this and bonus question, shall we have the business date 
here, the tenant datetime or the system datetime? (pointer: DateUtils class)



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableReportingProcessService.java:
##########
@@ -90,13 +112,64 @@ private Response exportJSON(String reportName, 
MultivaluedMap<String, String> qu
         return 
Response.ok().entity(json).type(MediaType.APPLICATION_JSON).build();
     }
 
-    private Response exportS3(String reportName, MultivaluedMap<String, 
String> queryParams, Map<String, String> reportParams,
-            boolean isSelfServiceUserReport, String parameterTypeValue) {
-        throw new UnsupportedOperationException("S3 export not supported for 
datatables");
+    private boolean isAwsCredentialsValid() {
+        return 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_ACCESS_KEY_ID.environmentVariable()))
+                && 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_SECRET_ACCESS_KEY.environmentVariable()))
+                && 
StringUtils.isNotBlank(SdkSystemSetting.AWS_REGION.environmentVariable());
+    }
+
+    private Response exportS3(String reportName, Map<String, String> 
reportParams, boolean isSelfServiceUserReport,
+            String parameterTypeValue) {
+        if (!isAwsCredentialsValid()) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS 
credentials not set").build();
+        }
+        if (StringUtils.isBlank(System.getenv().get(AWS_S_3_BUCKET_NAME))) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS S3 
bucket name not set").build();
+        }
+        try {
+            S3Client s3Client = 
S3Client.builder().region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())))
+                    
.credentialsProvider(EnvironmentVariableCredentialsProvider.create()).build();
+            StreamingOutput output = 
this.readExtraDataAndReportingService.retrieveReportCSV(reportName, 
parameterTypeValue, reportParams,
+                    isSelfServiceUserReport);
+            ByteArrayOutputStream byteArrayOutputStream = new 
ByteArrayOutputStream();
+            output.write(byteArrayOutputStream);
+            byteArrayOutputStream.close();
+            String finalS3Path = getS3FolderName() + "" + 
exportFileNameGeneration(reportName, reportParams) + ".csv";
+            s3Client.putObject(builder -> 
builder.bucket(System.getenv().get(AWS_S_3_BUCKET_NAME)).key(finalS3Path).build(),
+                    
RequestBody.fromBytes(byteArrayOutputStream.toByteArray()));
+            return Response.noContent().build();
+        } catch (IOException e) {
+            log.error("Error while exporting to S3", e);
+            throw new IllegalStateException("Error while exporting to S3!", e);
+        }
+    }
+
+    private String getS3FolderName() {
+        String folderName = 
configurationDomainService.retrieveReportExportS3FolderName();
+        if (StringUtils.isNotBlank(folderName)) {
+            folderName = folderName + "/";
+        }
+        return folderName.trim();

Review Comment:
   why don't we trim beforehand? the end will not be trimmed anyway because you 
append a `/` at the end.
   I think I'm missing more validation here since this is a user editable value 
(the global config). I think the configurationDomainService should take care of 
validating whether the folder name doesn't contain a slash at the end and 
whitespaces anywhere so that from a contractual point of view, whoever uses the 
`retrieveReportExportS3FolderName` method, they don't need to do the same check 
over and over again.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableReportingProcessService.java:
##########
@@ -90,13 +112,64 @@ private Response exportJSON(String reportName, 
MultivaluedMap<String, String> qu
         return 
Response.ok().entity(json).type(MediaType.APPLICATION_JSON).build();
     }
 
-    private Response exportS3(String reportName, MultivaluedMap<String, 
String> queryParams, Map<String, String> reportParams,
-            boolean isSelfServiceUserReport, String parameterTypeValue) {
-        throw new UnsupportedOperationException("S3 export not supported for 
datatables");
+    private boolean isAwsCredentialsValid() {
+        return 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_ACCESS_KEY_ID.environmentVariable()))
+                && 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_SECRET_ACCESS_KEY.environmentVariable()))
+                && 
StringUtils.isNotBlank(SdkSystemSetting.AWS_REGION.environmentVariable());
+    }
+
+    private Response exportS3(String reportName, Map<String, String> 
reportParams, boolean isSelfServiceUserReport,
+            String parameterTypeValue) {
+        if (!isAwsCredentialsValid()) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS 
credentials not set").build();
+        }
+        if (StringUtils.isBlank(System.getenv().get(AWS_S_3_BUCKET_NAME))) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS S3 
bucket name not set").build();
+        }
+        try {
+            S3Client s3Client = 
S3Client.builder().region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())))
+                    
.credentialsProvider(EnvironmentVariableCredentialsProvider.create()).build();
+            StreamingOutput output = 
this.readExtraDataAndReportingService.retrieveReportCSV(reportName, 
parameterTypeValue, reportParams,
+                    isSelfServiceUserReport);
+            ByteArrayOutputStream byteArrayOutputStream = new 
ByteArrayOutputStream();
+            output.write(byteArrayOutputStream);
+            byteArrayOutputStream.close();
+            String finalS3Path = getS3FolderName() + "" + 
exportFileNameGeneration(reportName, reportParams) + ".csv";
+            s3Client.putObject(builder -> 
builder.bucket(System.getenv().get(AWS_S_3_BUCKET_NAME)).key(finalS3Path).build(),
+                    
RequestBody.fromBytes(byteArrayOutputStream.toByteArray()));
+            return Response.noContent().build();
+        } catch (IOException e) {
+            log.error("Error while exporting to S3", e);
+            throw new IllegalStateException("Error while exporting to S3!", e);
+        }
+    }
+
+    private String getS3FolderName() {
+        String folderName = 
configurationDomainService.retrieveReportExportS3FolderName();
+        if (StringUtils.isNotBlank(folderName)) {
+            folderName = folderName + "/";
+        }
+        return folderName.trim();
+    }
+
+    private String exportFileNameGeneration(String reportName, Map<String, 
String> reportParams) {
+        String timestamp = "_" + new 
SimpleDateFormat("yyyyMMddHHmmss").format(new Date());
+        int reportMaximumFileName = 512 - timestamp.length() - 4;

Review Comment:
   Comment here please on where the 512 and 4 comes from.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/dataqueries/service/DatatableReportingProcessService.java:
##########
@@ -90,13 +112,64 @@ private Response exportJSON(String reportName, 
MultivaluedMap<String, String> qu
         return 
Response.ok().entity(json).type(MediaType.APPLICATION_JSON).build();
     }
 
-    private Response exportS3(String reportName, MultivaluedMap<String, 
String> queryParams, Map<String, String> reportParams,
-            boolean isSelfServiceUserReport, String parameterTypeValue) {
-        throw new UnsupportedOperationException("S3 export not supported for 
datatables");
+    private boolean isAwsCredentialsValid() {
+        return 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_ACCESS_KEY_ID.environmentVariable()))
+                && 
StringUtils.isNotBlank(System.getenv(SdkSystemSetting.AWS_SECRET_ACCESS_KEY.environmentVariable()))
+                && 
StringUtils.isNotBlank(SdkSystemSetting.AWS_REGION.environmentVariable());
+    }
+
+    private Response exportS3(String reportName, Map<String, String> 
reportParams, boolean isSelfServiceUserReport,
+            String parameterTypeValue) {
+        if (!isAwsCredentialsValid()) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS 
credentials not set").build();
+        }
+        if (StringUtils.isBlank(System.getenv().get(AWS_S_3_BUCKET_NAME))) {
+            return Response.status(Response.Status.BAD_REQUEST).entity("AWS S3 
bucket name not set").build();
+        }
+        try {
+            S3Client s3Client = 
S3Client.builder().region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())))
+                    
.credentialsProvider(EnvironmentVariableCredentialsProvider.create()).build();
+            StreamingOutput output = 
this.readExtraDataAndReportingService.retrieveReportCSV(reportName, 
parameterTypeValue, reportParams,
+                    isSelfServiceUserReport);
+            ByteArrayOutputStream byteArrayOutputStream = new 
ByteArrayOutputStream();
+            output.write(byteArrayOutputStream);
+            byteArrayOutputStream.close();
+            String finalS3Path = getS3FolderName() + "" + 
exportFileNameGeneration(reportName, reportParams) + ".csv";
+            s3Client.putObject(builder -> 
builder.bucket(System.getenv().get(AWS_S_3_BUCKET_NAME)).key(finalS3Path).build(),
+                    
RequestBody.fromBytes(byteArrayOutputStream.toByteArray()));
+            return Response.noContent().build();
+        } catch (IOException e) {
+            log.error("Error while exporting to S3", e);
+            throw new IllegalStateException("Error while exporting to S3!", e);
+        }
+    }
+
+    private String getS3FolderName() {
+        String folderName = 
configurationDomainService.retrieveReportExportS3FolderName();
+        if (StringUtils.isNotBlank(folderName)) {
+            folderName = folderName + "/";
+        }
+        return folderName.trim();
+    }
+
+    private String exportFileNameGeneration(String reportName, Map<String, 
String> reportParams) {
+        String timestamp = "_" + new 
SimpleDateFormat("yyyyMMddHHmmss").format(new Date());
+        int reportMaximumFileName = 512 - timestamp.length() - 4;
+        String fileName = reportName;
+        if (reportParams != null) {

Review Comment:
   This should really be extracted into another class & method to be able to 
properly unit test it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to