Aggies sprint10 staging #127

kaladay · 2024-11-11T14:01:14Z

- FOLIO uses 2-space spacing/tabbing. - Use the `@Mock` annotation whenever possible. - Restore the MockMVC Constants for mocking the TENANT and TOKEN provided by the spring-module-core test project.

…stent. The patterns should use `workflow-engine` for the engine. The engine happens to be Camunda in this particular case but this path should not be tied to Camunda. The module name should be in there as well, which would be `mod-workflow`. The combination does not need to be `mod-workflow-workflow-engine` as that seems silly. Instead, the second `-workflow` is dropped.

…patterns. Use the `@BeforeEach`. - I suspect that the `ReflectionTestUtils.setField...` could be defined as a `@BeforeAll`, but I decided to withhold going further at this time. The standard practice is to use `provide...` for the stream provider method names. Then, the generic `Stream.of()` are used rather than using explicit types. Next, the conditional methods are added as a parameter. - The conditional parameters will allow us to add more extensive tests using the same provider in the future.

The Camunda base path and the Camunda reset path are needed. When going through OKAPI REST end points, these paths represent the OKAPI REST path. This is not necessarily a path to where mod-camunda actually listens on. This commit puts the code back in a state where the Camunda OKAPI REST paths are less hardcoded. Remove the code smells regarding logging an error and throwing an exception at the same time that was introduced when the path changes were made. Update the debug log to show both the tenant and token regardless of whether or not they are NULL. Also use a single message to reduce the amount of log spamming that can happen due to this being called on every request. Update the `application.yml` from the tests to more closely match the `application.yml` from the normal build. The changes being reverted introduced problems and are outside the scope of a change from ActiveMQ to Kafka. The `restPath` variables should probably be refactored to the `workflow-engine` rather than `rest`. However, this has been out of scope and there are other issues set aside for handling this.

- Add missing environment variables. - Add environment variables that may have a significant impact on the OKAPI REST usage, such as `OKAPI_CAMUNDA_BASEPATH` and `OKAPI_CAMUNDA_RESTPATH`. - Add missing periods in several of the descriptions.

The `openapi.xml` file is rebuilt using `mvn clean verify -P openapi`. Changes to the module descriptor or the controller end points require rebuilding of the OpenAPI file.

The `workflow-engine` is what needs to be used. However, that is a different issue and is out of scope. Leave this as `camunda` to stay within the scope of the issue being resolved.

… name.

Fix the permissions section and clean up the environment variable table.

This only provides `prePersists()` tests for the following classes: - `AbstractGateway` - `AbstractProcess` - `AbstractTask` - `CompressFile` - `EmbeddedInput`

MODWRKFLOW-34: Mod Workflow should support kafka messaging instead of active mq.

Added extra line as per suggestion

This can be tested by doing the following: ```shell mvn clean test jacoco:report firefox components/target/site/jacoco/index.html firefox service/target/site/jacoco/index.html ```

Add support for Jacoco to allow for local coverage reports.

Directory task unit testing

Add unit test for prePersist() from EmbeddedInput.

…onTest.java Added to a new line Co-authored-by: Kevin Day <[email protected]>

…ument params

Unit tests created for prePersist()

Add additional unit tests for "has" functions for EmbeddedLoopReference.

The `openapi.xml` file is rebuilt using `mvn clean verify -P openapi`.

wwelling · 2024-11-11T19:42:51Z

README.md

+| DB_QUERYTIMEOUT                |           60000             | Database query timeout.                                                                                                                                    |
+| DB_USERNAME                    |        folio_admin          | Postgres user name.                                                                                                                                        |
+| JAVA_OPTIONS                   | `-XX:MaxRAMPercentage=75.0` | Java options.                                                                                                                                              |
+| KAFKA_HOST                     |           kafka             | Kafka broker host name.                                                                                                                                    |


https://english.stackexchange.com/questions/100858/hostname-or-host-name#:~:text=Hostname%20specifically%20refers%20to%20a,a%20specific%20setting%20of%20it.

service/src/test/resources/application.yaml

wwelling · 2024-11-11T19:58:05Z

service/src/main/java/org/folio/rest/workflow/service/OkapiDiscoveryService.java

@@ -91,7 +94,7 @@ public Map<String, List<Handler>> getHandlers(String tenant, String id) throws I
  }

  public JsonNode getModules(String tenant) {
-    String url = okapiUrl + PROXY_TENANT_BASE_PATH + tenant + MODULES_SUB_PATH;
+    String url = okapiUrl + basePath + PROXY_TENANT_BASE_PATH + tenant + MODULES_SUB_PATH;


This is not correct. See line 33 and 34. Okapi requests to /_/proxy/tenants/ and /_/proxy/modules/ will be incorrect if prefexid with mod-camunda base-path anything other than /. Even then a URL with consecutive double forward slashes is concatenated.

Additional, okapiUrl should have any trailing forward slash removed to be safe.

wwelling · 2024-11-12T15:23:14Z

service/src/test/resources/application.yaml

  domain-packages:
    - org.folio.rest.workflow.components
    - org.folio.rest.workflow.model
  schema-scripts:

 okapi:
  url: ${OKAPI_URL:http://localhost:9130}
+  camunda:


Could you help explain again why both of these paths are required for mod-workflow to communicate with mod-camunda through Okapi? Is there some limitation with Camunda Dashboard or its configuration for the Camunda API?

kaladay and others added 30 commits October 28, 2024 09:51

MODWRKFLOW-34: Fix style practice and restore proper mocking.

76da95b

- FOLIO uses 2-space spacing/tabbing. - Use the `@Mock` annotation whenever possible. - Restore the MockMVC Constants for mocking the TENANT and TOKEN provided by the spring-module-core test project.

MODWRKFLOW-34: Remove activemq references from git ignore file.

6099ee0

MODWRKFLOW-34: Regenerate the OpenAPI file.

3d6dc3e

The `openapi.xml` file is rebuilt using `mvn clean verify -P openapi`. Changes to the module descriptor or the controller end points require rebuilding of the OpenAPI file.

MODWRKFLOW-34: Use camunda instead of workflow-engine for now for Kafka.

ca68e45

The `workflow-engine` is what needs to be used. However, that is a different issue and is out of scope. Leave this as `camunda` to stay within the scope of the issue being resolved.

MODWRKFLOW-34: The KAFKA_HOST environment variable is a URL and not a…

bfea9a6

… name.

MODWRKFLOW-34: Update the readme.

7077198

Fix the permissions section and clean up the environment variable table.

Add several unit tests for prePersist() methods.

2ef0ed7

This only provides `prePersists()` tests for the following classes: - `AbstractGateway` - `AbstractProcess` - `AbstractTask` - `CompressFile` - `EmbeddedInput`

Unit tests written for DirectoryTask following the AbstractGateway tests

00da790

code clean up

a1dc860

MODWRKFLOW-34: Remove unused activemq.version property.

b528514

Merge pull request #122 from folio-org/MODWRKFLOW-34

4f08484

MODWRKFLOW-34: Mod Workflow should support kafka messaging instead of active mq.

Update DirectoryTaskTest.java

c9b9538

Added extra line as per suggestion

Add support for Jacoco to allow for local coverage reports.

bf295b5

This can be tested by doing the following: ```shell mvn clean test jacoco:report firefox components/target/site/jacoco/index.html firefox service/target/site/jacoco/index.html ```

Merge pull request #124 from folio-org/aggies-sprint10-jacoco

ba25ce9

Add support for Jacoco to allow for local coverage reports.

Merge pull request #121 from folio-org/directory-task-unit-testing

eb72c9b

Directory task unit testing

DirectoryTask is not abstract so do not use Impl helper class.

7899977

Merge pull request #120 from folio-org/unit_test_coverage-kevin

f48a374

Add unit test for prePersist() from EmbeddedInput.

Added unit test for prePersist() for Condition class

638303c

Added unit test for prePersist() for ConnectTo class

4e7ef34

Added unit test for prePersist() for DatabaseConnectionTask

e751a47

Added unit tests for prePersist()

27f7cb7

Added unit test covering prePersist() for EmailTask

e8851d9

Unit tests created for prePersist()

54382dc

Update components/src/test/java/org/folio/rest/workflow/model/Conditi…

d6e4341

…onTest.java Added to a new line Co-authored-by: Kevin Day <[email protected]>

Unit tests created for prePersist - EmbeddedProcessorTest

78ea00c

Unit tests created for prePersist() - EmbeddedRequestTest

cde6405

rmathew1011 and others added 5 commits November 8, 2024 16:19

Added unit test for prePersist(), added constants and aligned the Arg…

d1684ef

…ument params

Merge pull request #125 from folio-org/unit-tests

9a9609c

Unit tests created for prePersist()

Add additional unit tests for "has" functions for EmbeddedLoopReference.

934e641

Merge pull request #126 from folio-org/aggies-sprint10-unit_tests

a8e02fc

Add additional unit tests for "has" functions for EmbeddedLoopReference.

Regenerate the OpenAPI file.

ef24074

The `openapi.xml` file is rebuilt using `mvn clean verify -P openapi`.

kaladay requested a review from a team November 11, 2024 16:12

wwelling reviewed Nov 11, 2024

View reviewed changes

service/src/test/resources/application.yaml Outdated Show resolved Hide resolved

wwelling reviewed Nov 11, 2024

View reviewed changes

kaladay added 2 commits November 11, 2024 15:39

Fix grammar where username should be user and name.

173614f

Further reword comment, removing Kafka entirely.

e299e4e

wwelling reviewed Nov 12, 2024

View reviewed changes

qtamu approved these changes Nov 13, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Aggies sprint10 staging #127

Aggies sprint10 staging #127

kaladay commented Nov 11, 2024

wwelling Nov 11, 2024

wwelling Nov 12, 2024

wwelling Nov 11, 2024 •

edited

Loading

wwelling Nov 12, 2024

	\| KAFKA_HOST \| kafka \| Kafka broker host name. \|
	\| KAFKA_HOST \| kafka \| Kafka broker hostname. \|

Aggies sprint10 staging #127

Are you sure you want to change the base?

Aggies sprint10 staging #127

Conversation

kaladay commented Nov 11, 2024

wwelling Nov 11, 2024

Choose a reason for hiding this comment

wwelling Nov 12, 2024

Choose a reason for hiding this comment

wwelling Nov 11, 2024 • edited Loading

Choose a reason for hiding this comment

wwelling Nov 12, 2024

Choose a reason for hiding this comment

wwelling Nov 11, 2024 •

edited

Loading