galovics commented on code in PR #2384:
URL: https://github.com/apache/fineract/pull/2384#discussion_r909522453
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/security/filter/TenantAwareTenantIdentifierFilter.java:
##########
@@ -66,18 +70,21 @@ public class TenantAwareTenantIdentifierFilter extends
GenericFilterBean {
private final ConfigurationDomainService configurationDomainService;
private final CacheWritePlatformService cacheWritePlatformService;
+ private final BusinessDateReadPlatformService
businessDateReadPlatformService;
+
private final String tenantRequestHeader = "Fineract-Platform-TenantId";
private final boolean exceptionIfHeaderMissing = true;
private final String apiUri = "/api/v1/";
@Autowired
public TenantAwareTenantIdentifierFilter(final
BasicAuthTenantDetailsService basicAuthTenantDetailsService,
final ToApiJsonSerializer<PlatformRequestLog> toApiJsonSerializer,
final ConfigurationDomainService configurationDomainService,
- final CacheWritePlatformService cacheWritePlatformService) {
+ final CacheWritePlatformService cacheWritePlatformService,
BusinessDateReadPlatformService businessDateReadPlatformService) {
Review Comment:
`@RequiredArgsConstructor`?
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/service/SchedulerTriggerListener.java:
##########
@@ -42,11 +46,14 @@ public class SchedulerTriggerListener implements
TriggerListener {
private final SchedularWritePlatformService schedularService;
private final TenantDetailsService tenantDetailsService;
+ private final BusinessDateReadPlatformService
businessDateReadPlatformService;
+
@Autowired
- public SchedulerTriggerListener(final SchedularWritePlatformService
schedularService, final TenantDetailsService tenantDetailsService) {
+ public SchedulerTriggerListener(final SchedularWritePlatformService
schedularService, final TenantDetailsService tenantDetailsService,
Review Comment:
`@RequiredArgsConstructor`?
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/ThreadLocalContextUtil.java:
##########
@@ -63,12 +64,55 @@ public static void clearDataSourceContext() {
contextHolder.remove();
}
+ public static String getAuthToken() {
+ return authTokenContext.get();
+ }
+
public static void setAuthToken(final String authToken) {
authTokenContext.set(authToken);
}
- public static String getAuthToken() {
- return authTokenContext.get();
+ public static HashMap<BusinessDateType, LocalDate> getBusinessDates() {
Review Comment:
No `HashMap` here either, simple `Map` is fine.
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/ThreadLocalContextUtil.java:
##########
@@ -26,29 +31,25 @@
*/
public final class ThreadLocalContextUtil {
- private ThreadLocalContextUtil() {
-
- }
-
public static final String CONTEXT_TENANTS = "tenants";
-
private static final ThreadLocal<String> contextHolder = new
ThreadLocal<>();
-
- private static final ThreadLocal<FineractPlatformTenant> tenantcontext =
new ThreadLocal<>();
-
+ private static final ThreadLocal<FineractPlatformTenant> tenantContext =
new ThreadLocal<>();
private static final ThreadLocal<String> authTokenContext = new
ThreadLocal<>();
+ private static final ThreadLocal<HashMap<BusinessDateType, LocalDate>>
businessDateContext = new ThreadLocal<>();
Review Comment:
Do you think we could switch the `HashMap` into a regular `Map` interface?
I'd say let's try to be as independent from a specific implementation as
possible.
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/PortfolioAccountReadPlatformServiceImpl.java:
##########
@@ -294,18 +294,22 @@ public PortfolioAccountData mapRow(final ResultSet rs,
@SuppressWarnings("unused
private static final class PortfolioLoanAccountRefundByTransferMapper
implements RowMapper<PortfolioAccountData> {
- private final String schemaSql;
+ private String schemaSql;
+ private final DatabaseSpecificSQLGenerator sqlGenerator;
PortfolioLoanAccountRefundByTransferMapper(DatabaseSpecificSQLGenerator
sqlGenerator) {
+ this.sqlGenerator = sqlGenerator;
Review Comment:
`@RequiredArgsConstructor`?
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/ThreadLocalContextUtil.java:
##########
@@ -63,12 +64,55 @@ public static void clearDataSourceContext() {
contextHolder.remove();
}
+ public static String getAuthToken() {
+ return authTokenContext.get();
+ }
+
public static void setAuthToken(final String authToken) {
authTokenContext.set(authToken);
}
- public static String getAuthToken() {
- return authTokenContext.get();
+ public static HashMap<BusinessDateType, LocalDate> getBusinessDates() {
+ Assert.notNull(businessDateContext.get(), "Business dates cannot be
null!");
+ return businessDateContext.get();
+ }
+
+ public static void setBusinessDates(HashMap<BusinessDateType, LocalDate>
dates) {
+ Assert.notNull(dates, "Business dates cannot be null!");
+ businessDateContext.set(dates);
+ }
+
+ public static LocalDate getBusinessDateByType(BusinessDateType
businessDateType) {
+ Assert.notNull(businessDateType, "Business date type cannot be null!");
+ LocalDate localDate = getBusinessDates().get(businessDateType);
+ Assert.notNull(localDate, String.format("Business date with type `%s`
is not initialised!", businessDateType));
+ return localDate;
+ }
+
+ public static LocalDate getBusinessDate() {
+ BusinessDateType businessDateType =
getActionContext().getBusinessDateType();
+ return getBusinessDateByType(businessDateType);
}
+ public static ActionContext getActionContext() {
+ return actionContext.get() == null ? ActionContext.DEFAULT :
actionContext.get();
+ }
+
+ public static void setActionContext(ActionContext context) {
+ Assert.notNull(context, "context cannot be null");
+ actionContext.set(context);
+ }
+
+ public static FineractContext syncDown() {
+ return new FineractContext(getDataSourceContext(), getTenant(),
getAuthToken(), getBusinessDates(), getActionContext());
+ }
+
+ public static void syncUp(final FineractContext fineractContext) {
Review Comment:
Same as above. Probably fromContext would be a better naming.
##########
fineract-provider/src/main/java/org/apache/fineract/adhocquery/service/AdHocScheduledJobRunnerServiceImpl.java:
##########
@@ -42,12 +42,14 @@ public class AdHocScheduledJobRunnerServiceImpl implements
AdHocScheduledJobRunn
private static final Logger LOG =
LoggerFactory.getLogger(AdHocScheduledJobRunnerServiceImpl.class);
private final AdHocReadPlatformService adHocReadPlatformService;
private final JdbcTemplate jdbcTemplate;
+ private final DatabaseSpecificSQLGenerator sqlGenerator;
@Autowired
- public AdHocScheduledJobRunnerServiceImpl(final JdbcTemplate jdbcTemplate,
final AdHocReadPlatformService adHocReadPlatformService) {
+ public AdHocScheduledJobRunnerServiceImpl(final JdbcTemplate jdbcTemplate,
final AdHocReadPlatformService adHocReadPlatformService,
Review Comment:
Could be replaced with `@RequiredArgsConstructor`
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/ThreadLocalContextUtil.java:
##########
@@ -63,12 +64,55 @@ public static void clearDataSourceContext() {
contextHolder.remove();
}
+ public static String getAuthToken() {
+ return authTokenContext.get();
+ }
+
public static void setAuthToken(final String authToken) {
authTokenContext.set(authToken);
}
- public static String getAuthToken() {
- return authTokenContext.get();
+ public static HashMap<BusinessDateType, LocalDate> getBusinessDates() {
+ Assert.notNull(businessDateContext.get(), "Business dates cannot be
null!");
+ return businessDateContext.get();
+ }
+
+ public static void setBusinessDates(HashMap<BusinessDateType, LocalDate>
dates) {
+ Assert.notNull(dates, "Business dates cannot be null!");
+ businessDateContext.set(dates);
+ }
+
+ public static LocalDate getBusinessDateByType(BusinessDateType
businessDateType) {
+ Assert.notNull(businessDateType, "Business date type cannot be null!");
+ LocalDate localDate = getBusinessDates().get(businessDateType);
+ Assert.notNull(localDate, String.format("Business date with type `%s`
is not initialised!", businessDateType));
+ return localDate;
+ }
+
+ public static LocalDate getBusinessDate() {
+ BusinessDateType businessDateType =
getActionContext().getBusinessDateType();
+ return getBusinessDateByType(businessDateType);
}
+ public static ActionContext getActionContext() {
+ return actionContext.get() == null ? ActionContext.DEFAULT :
actionContext.get();
+ }
+
+ public static void setActionContext(ActionContext context) {
+ Assert.notNull(context, "context cannot be null");
+ actionContext.set(context);
+ }
+
+ public static FineractContext syncDown() {
Review Comment:
`syncDown` is a bit confusing for me cause it's not "syncing" anything per
se. I'd simply call it something like
`ThreadLocalContextUtil#getContext`/`ThreadLocalContextUtil#toContext`
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/service/SchedulerJobListener.java:
##########
@@ -39,25 +43,23 @@
/**
* Global job Listener class to set Tenant details to {@link
ThreadLocalContextUtil} for batch Job and stores the batch
* job status to database after the execution
- *
*/
@Component
public class SchedulerJobListener implements JobListener {
- private int stackTraceLevel = 0;
-
private final String name =
SchedulerServiceConstants.DEFAULT_LISTENER_NAME;
-
private final SchedularWritePlatformService schedularService;
-
private final AppUserRepositoryWrapper userRepository;
-
private final GrantedAuthoritiesMapper authoritiesMapper = new
NullAuthoritiesMapper();
+ private final BusinessDateReadPlatformService
businessDateReadPlatformService;
+ private int stackTraceLevel = 0;
@Autowired
- public SchedulerJobListener(final SchedularWritePlatformService
schedularService, final AppUserRepositoryWrapper userRepository) {
+ public SchedulerJobListener(final SchedularWritePlatformService
schedularService, final AppUserRepositoryWrapper userRepository,
Review Comment:
`@RequiredArgsConstructor`?
##########
fineract-provider/src/main/java/org/apache/fineract/adhocquery/service/AdHocScheduledJobRunnerServiceImpl.java:
##########
@@ -42,12 +42,14 @@ public class AdHocScheduledJobRunnerServiceImpl implements
AdHocScheduledJobRunn
private static final Logger LOG =
LoggerFactory.getLogger(AdHocScheduledJobRunnerServiceImpl.class);
Review Comment:
I know you haven't introduced it but if you're touching the constructor
already, we could replace this with an `@Slf4J`
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/FineractContext.java:
##########
@@ -0,0 +1,37 @@
+/**
+ * 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.core.domain;
+
+import java.io.Serializable;
+import java.time.LocalDate;
+import java.util.HashMap;
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import org.apache.fineract.infrastructure.businessdate.domain.BusinessDateType;
+
+@AllArgsConstructor
+@Getter
+public class FineractContext implements Serializable {
+
+ private final String contextHolder;
+ private final FineractPlatformTenant tenantContext;
+ private final String authTokenContext;
+ private final HashMap<BusinessDateType, LocalDate> businessDateContext;
Review Comment:
Simple Map here too?
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/sms/scheduler/SmsMessageScheduledJobServiceImpl.java:
##########
@@ -65,14 +65,14 @@
@Service
public class SmsMessageScheduledJobServiceImpl implements
SmsMessageScheduledJobService {
+ private static final Logger LOG =
LoggerFactory.getLogger(SmsMessageScheduledJobServiceImpl.class);
Review Comment:
`@Slf4j`?
--
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]
