BITMAG-1247-date-time-modernizing by Knud-Aage · Pull Request #82 · bitrepository/reference

Knud-Aage · 2026-03-27T10:14:43Z

This PR addresses the long-standing technical debt of using legacy Java date/time classes.

The legacy date/time API is confusing, error-prone, mutable, and not thread-safe. It has been replaced by modern, immutable, thread-safe, and highly explicit API

Removal of XMLGregorianCalendar, though, awaits clarification, but will be done as soon as possible

- Added JUnit 5 annotations to BitrepositoryTestSuite - Added GlobalSuiteExtension to implement JUnit 5 extension points for suite-level setup and teardown. - Ensured consistency and correctness of test suite configuration and setup methods. This commit updates the test suite configuration to use JUnit 5 annotations, allowing for more flexible and powerful test suite management.

…-testng-with-junit

…junit' into BITMAG-1244-junit-bitrepository-core # Conflicts: # bitrepository-core/src/test/java/org/bitrepository/protocol/IntegrationTest.java # bitrepository-core/src/test/java/org/bitrepository/protocol/bus/ActiveMQMessageBusTest.java # bitrepository-core/src/test/java/org/bitrepository/protocol/performancetest/MessageBusNumberOfListenersStressTest.java

…n choose to include the test or not

…n install can't run because it gives errors in the module bitrepository-client, which hasn't been converted to junit 5 yet.

…testng-junit-all

…tead of TestNG

…use the codebase is now using junit since there was a security vulnerability with TestNG

…tests.

…accept # Conflicts: # bitrepository-core/src/test/java/org/bitrepository/protocol/IntegrationTest.java

…gured. The task is almost done but not there yet. To emulate jaccept a try to report during test as jaccept does, but that task is also not done yet.

…get/allure-results folder which are used to create the final report that are shown to the user. That report is created by using mvn allure:serve

…source folder were removed. Made minor corrections, so now all tests succeed. There are a few tests that were disabled.

…lureLifecycle errors.

…ls to use String.format instead of StringBuilder

…odern types instead.

…ndling.

…g' into BITMAG-1247-date-time-modernizing # Conflicts: # bitrepository-client/src/test/java/org/bitrepository/client/TestEventHandler.java # bitrepository-client/src/test/java/org/bitrepository/modify/deletefile/DeleteFileClientComponentTest.java # bitrepository-client/src/test/java/org/bitrepository/modify/deletefile/DeleteFileClientTestWrapper.java # bitrepository-core/src/main/java/org/bitrepository/common/utils/CalendarUtils.java # bitrepository-core/src/test/java/org/bitrepository/common/utils/FileUtilsTest.java # bitrepository-core/src/test/java/org/bitrepository/protocol/bus/ActiveMQMessageBusTest.java # bitrepository-core/src/test/java/org/bitrepository/protocol/bus/MessageReceiver.java # bitrepository-core/src/test/java/org/bitrepository/protocol/fileexchange/LocalFileExchangeTest.java # bitrepository-core/src/test/java/org/bitrepository/protocol/performancetest/MessageBusTimeToSendMessagesStressTest.java # bitrepository-integrity-service/src/test/java/org/bitrepository/integrityservice/component/IntegrityServiceComponentTest.java # bitrepository-reference-pillar/src/test/java/org/bitrepository/pillar/integration/PillarIntegrationTest.java # bitrepository-reference-pillar/src/test/java/org/bitrepository/pillar/integration/func/deletefile/DeleteFileRequestIT.java # bitrepository-reference-pillar/src/test/java/org/bitrepository/pillar/integration/func/getfile/GetFileRequestIT.java # bitrepository-reference-pillar/src/test/java/org/bitrepository/pillar/integration/func/putfile/IdentifyPillarsForPutFileIT.java # bitrepository-reference-pillar/src/test/java/org/bitrepository/pillar/integration/model/PillarFileManager.java # bitrepository-reference-pillar/src/test/resources/logback-test.xml # bitrepository-service/src/test/java/org/bitrepository/service/audit/AuditTrailContributorDatabaseMigrationTest.java

ole-v-v · 2026-03-31T16:48:16Z

Did you somehow forget the use of Date in the AuditTrailCollector and LocalAuditTrailPreserver classes? Also the RestIntegrityService class and classes under bitrepository-reference-pillar/src/test/java/org/bitrepository/pillar/messagefactories seem not to be fully converted yet.

Also some timer task code uses old methods. Since we already have an issue to rewrite our timer tasks, I suggest you can let it be for now.

ole-v-v

A strong pull request demonstrating a good command of java.time. We will be very happy about this change.

For naming new methods, when you add Instant to some previous method name. consider deleting Date or Time from the name to evade redundancy and the name becoming overly long.

ole-v-v · 2026-03-31T09:23:38Z

    private TimeZone localTimeZone = TimeZone.getDefault();
+    private ZoneId localZoneId = ZoneId.systemDefault();


Having both of a TimeZone and a ZoneId is redundant. I’d prefer just the modern ZoneId (with conversions as necessary). Granted, you are meticulously making sure that the two always refer to the same time zone, so the code works correctly also with the redundancy.

ole-v-v · 2026-03-31T10:22:00Z

     * @return The startDate;
     */
-    public Date getStartDate() {
+    public Instant getStartDateInstant() {


We could also just name it getStartInstant()? Or even just getStart()?? I believe an alarm starts at a point in time, so I don’t think the word “date” adds anything helpful here? Similarly for getEnd[Date]Instant().

Optional, if we want to take it a step further, we may rename constructor and method parameters and even instance variables to just start and end (leaving Date out from the names), or similar.

Changed the name to start and end

ole-v-v · 2026-03-31T10:37:43Z

@@ -44,12 +45,12 @@ public class AlarmDatabaseExtractionModel {
    /**
     * @see #getStartDate().


I suggest referring to the new getStartDateInstant() instead. Similarly for end instant below.

ole-v-v · 2026-03-31T10:39:10Z

+
    /**
     * @param startDate The startDate.
     * @see #getStartDate()


I suggest referring to the new getStartDateInstant() instead. Similarly for end instant below.

ole-v-v · 2026-03-31T11:57:04Z

+     * @param componentID  [OPTIONAL] The id of the component.
+     * @param alarmCode    [OPTIONAL] The alarm code.
+     * @param minDate      [OPTIONAL] The earliest date for the alarms.
+     * @param maxDate      [OPTIONAL] The latest date for the alarms.
+     * @param fileID       [OPTIONAL] The id of the file, which the alarms are connected.
+     * @param collectionID the ID of the collection. Perhaps it is optional
+     * @param count        [OPTIONAL] The maximum number of alarms to retrieve from the store.


You’ve got quite impressive Javadoc comments on methods that you have deprecated.

While the caller needs to know which parameters are allowed to be null, for my part I’m not fond of using all caps (except in abbreviations, when referring to constants declared in shouting case and similar). I would probably use wording along “The ID of the component or null for all components” (if this is the intended meaning; if you don’t know, then just “or null”).

Removed (OPTIONAL) and added with null

ole-v-v · 2026-04-08T10:38:55Z

+    public Instant getNextRunInstant(JobID jobId) {
+        if (intervalTasks.containsKey(jobId)) {
+            return intervalTasks.get(jobId).getNextRunInstant();
+        } else return null;


May use this occasion to put return statement on its own line enclosed in curly braces. Or depending on taste leave out else and just put the return statement on its own line.

ole-v-v · 2026-04-08T10:45:17Z

+        if (nextRun != null && (getNextRunInstant() == null ||
+                getNextRunInstant().toEpochMilli() <= System.currentTimeMillis())) {


I would prefer:

if (nextRun != null && (getNextRunInstant() == null || ! getNextRunInstant().isAfter(Instant.now()))) {

Breaking the line at a higher level of the Boolean expression and using “not after” to mean “before or equal to” (may also spell it out).

ole-v-v · 2026-04-08T10:50:17Z

    }

    /**
     * @return The duration of the workflow if it has been started, else 0.


May use this occasion for adding to the documentation that the duration is in milliseconds. Or even provide a method that returns a Duration object.

ole-v-v · 2026-04-08T10:51:13Z

+            return System.currentTimeMillis() - start.toEpochMilli();
        } else {
-            return finish.getTime() - start.getTime();
+            return finish.toEpochMilli() - start.toEpochMilli();


My suggestion:

return ChronoUnit.MILLIS.between(start, finish);

ole-v-v · 2026-04-08T10:55:34Z

+        DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSXXX", Locale.ROOT);
+        Instant summertimeTS = ZonedDateTime.parse("2015-10-25T02:59:54.000+02:00", formatter).toInstant();


Leave out the formatter and just do

Instant summertimeTS = OffsetDateTime.parse("2015-10-25T02:59:54.000+02:00").toInstant();

Same for the standard time case below. (This is testing java.time, which we should not want to consider our job; let it be.)

ole-v-v · 2026-04-10T09:24:55Z

+//    @Tag("regressiontest")
+//    public void onePillarRespondingWithPartialPutAllowed() throws Exception {
+//        addReference("<a href=https://sbforge.org/jira/browse/BITMAG-598>" +
+//                "BITMAG-598 It should be possible to putFiles, even though only a subset of the pillars are available</a>");
+//        addDescription("Tests the handling of missing identification responses from one pillar, " +
+//                "when partial put are allowed");
+//        addFixture("Sets the identification timeout to 100 ms and allow partial puts.");
+//
+//        settingsForCUT.getRepositorySettings().getClientSettings()
+//                .setIdentificationTimeoutDuration(datatypeFactory.newDuration(100));
+//        settingsForCUT.getReferenceSettings().getPutFileSettings().setPartialPutsAllow(true);
+//        TestEventHandler testEventHandler = new TestEventHandler();
+//        PutFileClient putClient = createPutFileClient();
+//
+//        addStep("Request the putting of a file through the PutClient",
+//                "A identification request should be dispatched.");
+//        putClient.putFile(collectionID, httpServerConfiguration.getURL(DEFAULT_FILE_ID), DEFAULT_FILE_ID, 0, null,
+//                null, testEventHandler, null);
+//        Assertions.assertEquals(OperationEventType.IDENTIFY_REQUEST_SENT, testEventHandler.waitForEvent().getEventType());
+//        IdentifyPillarsForPutFileRequest receivedIdentifyRequestMessage =
+//                collectionReceiver.waitForMessage(IdentifyPillarsForPutFileRequest.class);
+//


Right. Allow me to suggest you stick in a comment saying approximately what the above reply says. And then move on.

ole-v-v

A strong pull request demonstrating a good command of java.time. We will be very happy about this change.

For naming new methods, when you add Instant to some previous method name. consider deleting Date or Time from the name to evade redundancy and the name becoming overly long.

Knud-Aage added 30 commits December 11, 2025 14:01

Initial

d41038e

Merge remote-tracking branch 'origin/master' into BITMAG-1244-replace…

f50949f

…-testng-with-junit

Changes due to PR

7ec116c

Working but with errors in a few tests.

305d6fa

Put a @disabled at testManyTimes() and commented it out, so people ca…

155ad25

…n choose to include the test or not

The branch has now the same test methods that are wrong, but mvn clea…

8d421a4

…n install can't run because it gives errors in the module bitrepository-client, which hasn't been converted to junit 5 yet.

Able to test in junit

6d94dcf

Merge branch 'BITMAG-1244-junit-bitrepository-core' into BITMAG-1244-…

330c1ef

…testng-junit-all

Many tests migrated to junit but a log missing

56f5133

All tests succeeds after having been migrated from TestNG to JUnit 5

5888407

Removed the TestNG dependency

f96a705

The suite classes looks for tags instead of classes

7553d82

Modified setupPillarIntegrationTest method so it reflects junit 5 ins…

c565330

…tead of TestNG

Now closer to without testng

bbeace6

All the tests has been migrated to use allure instead of jaccept beca…

f38d907

…use the codebase is now using junit since there was a security vulnerability with TestNG

The disableness annotations have been commented out to enable stress …

86ee150

…tests.

Minor adjustments

dc838ff

Merge branch 'BITMAG-1244-testng-junit-all' into BITMAG-1244-remove-j…

6ddc6fa

…accept # Conflicts: # bitrepository-core/src/test/java/org/bitrepository/protocol/IntegrationTest.java

Removed allure errors and updated test

b8e8b1e

Changes to pom.xml to ensure allure tests folders are correctly confi…

c3ffb66

…gured. The task is almost done but not there yet. To emulate jaccept a try to report during test as jaccept does, but that task is also not done yet.

Changes due to PR review

c99b44c

Changes due to PR review

af01ae1

Changes due to PR review

c5a6865

Changes due to PR review

66b0dfb

Changes due to PR review

3ed247d

Changes due to PR review

ffb86f3

Changes due to PR review

321294c

Changes due to PR review

139bd5f

Knud-Aage added 21 commits March 31, 2026 16:39

Fixing merge errors

fd29397

Fixing merge errors

6eb1ff2

All the Allure reports are put in one folder in the bitrepository/tar…

b86e0cc

…get/allure-results folder which are used to create the final report that are shown to the user. That report is created by using mvn allure:serve

The stress tests that gave a lot of files in the reference pillars re…

89db8cf

…source folder were removed. Made minor corrections, so now all tests succeed. There are a few tests that were disabled.

Merged in AllureTestUtils improvements and added code for handling Al…

a86746b

…lureLifecycle errors.

Placed AllureTestUtils and AllureEventLogger in common/utils

5ef8d50

Importing Allure instead of io.qameta.allure and changed getAuditTrai…

5a31b31

…ls to use String.format instead of StringBuilder

Minor corrections and deleted MessageSenderStub.java

55357b2

Changes due to PR review

cd26ba3

Refactored code due to PR

622f225

Merged with master

635a15f

Changes due to PR

7fab4a4

Finished the date/time modernizing of all classes

5633dfd

More methods which deprecates XMLGregorianCalendar and Date and use m…

0dbae53

…odern types instead.

More classes updated to have a more modern date/time handling.

5c5e039

More classes updated to have a more modern date/time handling.

18fc4d7

More classes updated to have a more modern date/time handling.

e2c40ed

More classes updated or refactored to have a more modern date/time ha…

04d5a1f

…ndling.

Merged with master and made necessary updates

d227832

Deleted a }

7278d4f

ole-v-v requested changes Apr 8, 2026

View reviewed changes

Knud-Aage added 3 commits April 9, 2026 13:28

Changes due to PR

e4d3148

Inserted space after (Instant) if there were no one.

3a53c95

Changes due to PR

6ea62eb

ole-v-v reviewed Apr 10, 2026

View reviewed changes

bitrepository deleted a comment from ole-v-v Apr 10, 2026

Changes due to PR

1dfbfa2

ole-v-v requested changes Apr 14, 2026

View reviewed changes

		private TimeZone localTimeZone = TimeZone.getDefault();
		private ZoneId localZoneId = ZoneId.systemDefault();

		@@ -44,12 +45,12 @@ public class AlarmDatabaseExtractionModel {
		/**
		* @see #getStartDate().

		if (nextRun != null && (getNextRunInstant() == null \|\|
		getNextRunInstant().toEpochMilli() <= System.currentTimeMillis())) {

		DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSXXX", Locale.ROOT);
		Instant summertimeTS = ZonedDateTime.parse("2015-10-25T02:59:54.000+02:00", formatter).toInstant();

Conversation

Knud-Aage commented Mar 27, 2026

Uh oh!

ole-v-v commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ole-v-v left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ole-v-v Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ole-v-v left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ole-v-v commented Mar 31, 2026 •

edited

Loading

ole-v-v Apr 10, 2026 •

edited

Loading