vidakovic commented on code in PR #3994:
URL: https://github.com/apache/fineract/pull/3994#discussion_r1692562899


##########
custom/v3/commands/handler/src/main/java/org/apache/fineract/v3/commands/handler/NewTypedCommandSourceHandler.java:
##########
@@ -0,0 +1,27 @@
+/**
+ * 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.v3.commands.handler;
+
+import org.apache.fineract.infrastructure.core.data.CommandProcessingResult;
+import org.apache.fineract.v3.commands.data.JsonTypedCommand;
+
+public interface NewTypedCommandSourceHandler<T> {

Review Comment:
   Have to find a better name... tomorrow we might have another 
implementation... do we call it then "NewNewTypedCommandSourceHandler"? If in 
doubt always try to keep it as short and simple as possible: "CommandHandler"



##########
fineract-provider/src/main/resources/application.properties:
##########
@@ -44,6 +44,9 @@ 
fineract.tenant.read-only-password=${FINERACT_DEFAULT_TENANTDB_RO_PWD:}
 
fineract.tenant.read-only-parameters=${FINERACT_DEFAULT_TENANTDB_RO_CONN_PARAMS:}
 fineract.tenant.read-only-name=${FINERACT_DEFAULT_TENANTDB_RO_NAME:}
 
+# backwards compatibility for 
'org.apache.fineract.infrastructure.security.filter.TenantAwareBasicAuthenticationFilter'
+baseUrl=https://localhost:8443${FINERACT_SERVER_SERVLET_CONTEXT_PATH:/fineract-provider}

Review Comment:
   We need to use a variable for host and port... those are available already 
with Spring Boot (just don't remember the names). Should be used here instead 
of hardcoded values.



##########
custom/v3/commands/service/src/main/java/org/apache/fineract/v3/commands/service/SynchronousTypedCommandProcessingService.java:
##########
@@ -0,0 +1,102 @@
+/**
+ * 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.v3.commands.service;
+
+import io.github.resilience4j.retry.annotation.Retry;
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.commands.domain.CommandSource;
+import org.apache.fineract.commands.service.CommandSourceService;
+import org.apache.fineract.commands.service.IdempotencyKeyResolver;
+import org.apache.fineract.infrastructure.core.data.CommandProcessingResult;
+import 
org.apache.fineract.infrastructure.core.domain.BatchRequestContextHolder;
+import 
org.apache.fineract.infrastructure.core.domain.FineractRequestContextHolder;
+import org.apache.fineract.infrastructure.core.exception.ErrorHandler;
+import 
org.apache.fineract.infrastructure.jobs.service.SchedulerJobRunnerReadService;
+import 
org.apache.fineract.infrastructure.security.service.PlatformSecurityContext;
+import org.apache.fineract.useradministration.domain.AppUser;
+import org.apache.fineract.v3.commands.data.CommandTypedWrapper;
+import org.springframework.stereotype.Service;
+
+@Service
+@RequiredArgsConstructor
+public class SynchronousTypedCommandProcessingService implements 
TypedCommandProcessingService {
+
+    public static final String IDEMPOTENCY_KEY_STORE_FLAG = 
"idempotencyKeyStoreFlag";
+    public static final String COMMAND_SOURCE_ID = "commandSourceId";
+
+    private final PlatformSecurityContext context;
+    private final CommandSourceService commandSourceService;
+    private final IdempotencyKeyResolver idempotencyKeyResolver;
+    private final FineractRequestContextHolder fineractRequestContextHolder;
+    private final SchedulerJobRunnerReadService schedulerJobRunnerReadService;
+
+    @Override
+    public <T> CommandProcessingResult executeCommand(CommandTypedWrapper<T> 
wrapper) {

Review Comment:
   This still looks overly complicated and very similar to the current 
implementation. This is still very hard to reason about. I think we can do 
better and avoid these nested calls.



##########
custom/v3/commands/provider/src/main/java/org/apache/fineract/v3/commands/provider/NewCommandHandlerProvider.java:
##########
@@ -0,0 +1,95 @@
+/**
+ * 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.v3.commands.provider;
+
+import com.google.common.base.Preconditions;
+import java.util.HashMap;
+import lombok.NoArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.fineract.commands.annotation.CommandType;
+import org.apache.fineract.commands.exception.UnsupportedCommandException;
+import org.apache.fineract.v3.commands.handler.NewTypedCommandSourceHandler;
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.InitializingBean;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.ApplicationContextAware;
+import org.springframework.stereotype.Component;
+
+@Component
+@NoArgsConstructor
+@Slf4j
+public class NewCommandHandlerProvider implements ApplicationContextAware, 
InitializingBean {
+
+    private final HashMap<String, String> registeredHandlers = new HashMap<>();
+    private ApplicationContext applicationContext;
+
+    @Override
+    public void afterPropertiesSet() throws Exception {
+        initializeHandlerRegistry();
+    }
+
+    private void initializeHandlerRegistry() {
+        final String[] commandHandlerBeans = 
applicationContext.getBeanNamesForAnnotation(CommandType.class);
+        if (ArrayUtils.isNotEmpty(commandHandlerBeans)) {
+            for (final String commandHandlerName : commandHandlerBeans) {
+                log.debug("Register command handler '{}' ...", 
commandHandlerName);
+                final CommandType commandType = 
applicationContext.findAnnotationOnBean(commandHandlerName, CommandType.class);
+                try {
+                    if (commandType != null) {
+                        registeredHandlers.put(commandType.entity() + "|" + 
commandType.action(), commandHandlerName);
+                    } else {
+                        log.error("Unable to register command handler '{}'!", 
commandHandlerName);
+                    }
+                } catch (final Throwable th) {
+                    log.error("Unable to register command handler '{}'!", 
commandHandlerName, th);
+                }
+            }
+        }
+    }
+
+    /**
+     * Returns a handler for the given entity and action.<br>
+     * <br>
+     * Throws an {@link UnsupportedCommandException} if no handler for the 
given entity, action combination can be
+     * found.
+     *
+     * @param entity
+     *            the entity to lookup the handler, must be given.
+     * @param action
+     *            the action to lookup the handler, must be given.
+     */
+    @SuppressWarnings("unchecked")
+    public <T> NewTypedCommandSourceHandler<T> getHandler(final String entity, 
final String action) {
+        Preconditions.checkArgument(StringUtils.isNoneEmpty(entity), "An 
entity must be given!");

Review Comment:
   You don't need 2 libs (commons-lang, guava) to do these checks/assertions. 
Just use "Objects.requireNonNull()".



##########
custom/v3/commands/data/src/main/java/org/apache/fineract/v3/commands/data/JsonTypedCommand.java:
##########
@@ -0,0 +1,43 @@
+/**
+ * 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.v3.commands.data;
+
+import lombok.Builder;
+import lombok.Getter;
+
+@Getter
+@Builder
+public class JsonTypedCommand<T> {
+
+    private final T requestBody;
+    private final Long commandId;
+    private final Long resourceId;
+    private final Long subresourceId;
+    private final Long groupId;

Review Comment:
   Why are these IDs necessary here? This is mimicking exactly the same 
implementation as the current production one... in other words: if you have a 
command that has IDs belonging to different entities then it's not really type 
safe.



##########
custom/v3/commands/service/src/main/java/org/apache/fineract/v3/commands/service/CommandTypedWrapperBuilder.java:
##########
@@ -0,0 +1,115 @@
+/**
+ * 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.v3.commands.service;
+
+import lombok.AllArgsConstructor;
+import lombok.Builder;
+import lombok.Getter;
+import lombok.NoArgsConstructor;
+import org.apache.fineract.v3.commands.data.CommandTypedWrapper;
+
+@Getter
+@NoArgsConstructor
+@AllArgsConstructor
+@Builder(toBuilder = true)
+public class CommandTypedWrapperBuilder<T> {
+
+    private Long officeId;
+    private Long groupId;
+    private Long clientId;
+    private Long loanId;
+    private Long savingsId;
+    private String actionName;
+    private String entityName;
+    private Long entityId;
+    private Long subentityId;
+    private String href;
+    private T requestBody;
+    private String transactionId;
+    private Long productId;
+    private Long templateId;
+    private Long creditBureauId;
+    private Long organisationCreditBureauId;
+    private String jobName;
+    private String idempotencyKey;
+
+    public CommandTypedWrapper<T> buildCommandWrapper() {

Review Comment:
   Why is this function needed? How many times is it used (my bet: only once)? 
Why the slashes "//" at the end of the line?



##########
custom/v3/commands/provider/dependencies.gradle:
##########
@@ -0,0 +1,26 @@
+/**
+ * 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.
+ */
+
+dependencies {
+    implementation(project(':fineract-core'))
+    implementation(project(':custom:v3:commands:data'))
+    implementation(project(':custom:v3:commands:handler'))
+    implementation('org.apache.commons:commons-lang3')
+    implementation('com.google.guava:guava')

Review Comment:
   Why do we need this dependency (spoiler alert: assertions/validation of 
input values)? See comment below how to avoid.



##########
custom/v3/commands/data/src/main/java/org/apache/fineract/v3/commands/data/JsonTypedCommand.java:
##########
@@ -0,0 +1,43 @@
+/**
+ * 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.v3.commands.data;
+
+import lombok.Builder;
+import lombok.Getter;
+
+@Getter
+@Builder
+public class JsonTypedCommand<T> {

Review Comment:
   Why is "JSON" necessary here? Tomorrow we'll use GRPC or AVRO to serialize 
and then the naming will be off.



##########
custom/v3/note/service/src/main/java/org/apache/fineract/v3/note/service/NoteWritePlatformServiceImpl.java:
##########
@@ -0,0 +1,47 @@
+/**
+ * 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.v3.note.service;
+
+import org.apache.fineract.infrastructure.core.data.CommandProcessingResult;
+import org.apache.fineract.portfolio.client.domain.Client;
+import org.apache.fineract.v3.commands.data.JsonTypedCommand;
+import org.apache.fineract.v3.note.data.NoteRequestBody;
+
+public class NoteWritePlatformServiceImpl implements NoteWritePlatformService {
+
+    @Override
+    public CommandProcessingResult 
createNote(JsonTypedCommand<NoteRequestBody> command) {

Review Comment:
   Anything "JSON" is just wrong here...  this is what we wanted to avoid in 
the first place. ONLY DTOs!



##########
custom/v3/commands/service/src/main/java/org/apache/fineract/v3/commands/service/CommandTypedWrapperBuilder.java:
##########
@@ -0,0 +1,115 @@
+/**
+ * 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.v3.commands.service;
+
+import lombok.AllArgsConstructor;
+import lombok.Builder;
+import lombok.Getter;
+import lombok.NoArgsConstructor;
+import org.apache.fineract.v3.commands.data.CommandTypedWrapper;
+
+@Getter
+@NoArgsConstructor
+@AllArgsConstructor
+@Builder(toBuilder = true)
+public class CommandTypedWrapperBuilder<T> {
+
+    private Long officeId;
+    private Long groupId;
+    private Long clientId;
+    private Long loanId;
+    private Long savingsId;
+    private String actionName;
+    private String entityName;
+    private Long entityId;
+    private Long subentityId;
+    private String href;
+    private T requestBody;
+    private String transactionId;
+    private Long productId;
+    private Long templateId;
+    private Long creditBureauId;
+    private Long organisationCreditBureauId;
+    private String jobName;
+    private String idempotencyKey;
+
+    public CommandTypedWrapper<T> buildCommandWrapper() {
+        return CommandTypedWrapper.<T>builder() //
+                .officeId(this.officeId) //
+                .groupId(this.groupId) //
+                .clientId(this.clientId) //
+                .loanId(this.loanId) //
+                .savingsId(this.savingsId) //
+                .actionName(this.actionName) //
+                .entityName(this.entityName) //
+                .taskPermissionName(this.actionName + "_" + this.entityName) //
+                .entityId(this.entityId) //
+                .subentityId(this.subentityId) //
+                .href(this.href) //
+                .requestBody(this.requestBody) //
+                .transactionId(this.transactionId) //
+                .productId(this.productId) //
+                .templateId(this.templateId) //
+                .creditBureauId(this.creditBureauId) //
+                .organisationCreditBureauId(this.organisationCreditBureauId) //
+                .jobName(this.jobName) //
+                .idempotencyKey(this.idempotencyKey) //
+                .build(); //
+    }
+
+    public CommandTypedWrapperBuilder<T> createNote(final 
CommandTypedWrapper<T> resourceDetails, final String resourceType,

Review Comment:
   Is  it really necessary to have these functions here? How many times are 
they used... my bet is again: only once... in that case: don't put them here... 
we follow similar strategies all over the code and end up with 7000 LoC with 
utility functions like that... anti-pattern.



##########
fineract-core/src/main/java/org/apache/fineract/commands/service/PortfolioCommandSourceWritePlatformServiceImpl.java:
##########
@@ -53,9 +53,10 @@ public class PortfolioCommandSourceWritePlatformServiceImpl 
implements Portfolio
     @Override
     public CommandProcessingResult logCommandSource(final CommandWrapper 
wrapper) {

Review Comment:
   This looks again like the changes from another of your PRs... can't be 
accepted, because you duplicated code!!! One PR, one concern!



##########
custom/v3/note/api/src/main/java/org/apache/fineract/v3/note/api/NotesApiController.java:
##########
@@ -0,0 +1,172 @@
+/**
+ * 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.v3.note.api;
+
+import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
+import static 
org.springframework.http.MediaType.APPLICATION_PROBLEM_JSON_VALUE;
+
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import java.util.Collection;
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.portfolio.note.data.NoteData;
+import org.apache.fineract.portfolio.note.domain.NoteType;
+import 
org.apache.fineract.portfolio.note.exception.NoteResourceNotSupportedException;
+import org.apache.fineract.portfolio.note.service.NoteReadPlatformService;
+import org.apache.fineract.v3.commands.data.CommandTypedWrapper;
+import org.apache.fineract.v3.commands.service.CommandTypedWrapperBuilder;
+import org.apache.fineract.v3.note.data.NoteRequestBody;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.RequestBody;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RestController;
+
+@RestController

Review Comment:
   You are mixing read and write here... read related stuff is one PR, write 
related stuff is a separate PR... this cannot be approved, because this code is 
duplicated in 2 PRs!



##########
custom/v3/commands/service/src/main/java/org/apache/fineract/v3/commands/service/TypedCommandProcessingService.java:
##########
@@ -0,0 +1,28 @@
+/**
+ * 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.v3.commands.service;
+
+import org.apache.fineract.infrastructure.core.data.CommandProcessingResult;
+import org.apache.fineract.v3.commands.data.CommandTypedWrapper;
+
+public interface TypedCommandProcessingService {
+
+    <T> CommandProcessingResult executeCommand(CommandTypedWrapper<T> wrapper);

Review Comment:
   Where is the type safety for the "CommandProcessingResult"?



##########
custom/v3/note/api/src/main/java/org/apache/fineract/v3/note/api/NotesApiController.java:
##########
@@ -0,0 +1,172 @@
+/**
+ * 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.v3.note.api;
+
+import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
+import static 
org.springframework.http.MediaType.APPLICATION_PROBLEM_JSON_VALUE;
+
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import java.util.Collection;
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.portfolio.note.data.NoteData;
+import org.apache.fineract.portfolio.note.domain.NoteType;
+import 
org.apache.fineract.portfolio.note.exception.NoteResourceNotSupportedException;
+import org.apache.fineract.portfolio.note.service.NoteReadPlatformService;
+import org.apache.fineract.v3.commands.data.CommandTypedWrapper;
+import org.apache.fineract.v3.commands.service.CommandTypedWrapperBuilder;
+import org.apache.fineract.v3.note.data.NoteRequestBody;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.RequestBody;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RestController;
+
+@RestController

Review Comment:
   A cleaner approach would be to have 1 controller for read and a separate one 
for write... 



##########
custom/v3/commands/service/src/main/java/org/apache/fineract/v3/commands/service/SynchronousTypedCommandProcessingService.java:
##########
@@ -0,0 +1,102 @@
+/**
+ * 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.v3.commands.service;
+
+import io.github.resilience4j.retry.annotation.Retry;
+import lombok.RequiredArgsConstructor;
+import org.apache.fineract.commands.domain.CommandSource;
+import org.apache.fineract.commands.service.CommandSourceService;
+import org.apache.fineract.commands.service.IdempotencyKeyResolver;
+import org.apache.fineract.infrastructure.core.data.CommandProcessingResult;
+import 
org.apache.fineract.infrastructure.core.domain.BatchRequestContextHolder;
+import 
org.apache.fineract.infrastructure.core.domain.FineractRequestContextHolder;
+import org.apache.fineract.infrastructure.core.exception.ErrorHandler;
+import 
org.apache.fineract.infrastructure.jobs.service.SchedulerJobRunnerReadService;
+import 
org.apache.fineract.infrastructure.security.service.PlatformSecurityContext;
+import org.apache.fineract.useradministration.domain.AppUser;
+import org.apache.fineract.v3.commands.data.CommandTypedWrapper;
+import org.springframework.stereotype.Service;
+
+@Service
+@RequiredArgsConstructor
+public class SynchronousTypedCommandProcessingService implements 
TypedCommandProcessingService {

Review Comment:
   Naming can be improved. In other words: this is just too long... we don't 
have to reflect every feature of this implementation in the name.



-- 
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