DonalEvans commented on a change in pull request #5281:
URL: https://github.com/apache/geode/pull/5281#discussion_r444556037
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/control/SerializableRestoreRedundancyResultsImpl.java
##########
@@ -39,7 +39,8 @@
public void addPrimaryReassignmentDetails(PartitionRebalanceInfo details) {
totalPrimaryTransfersCompleted += details.getPrimaryTransfersCompleted();
totalPrimaryTransferTime =
- totalPrimaryTransferTime.plusMillis(details.getPrimaryTransferTime());
+ totalPrimaryTransferTime + details.getPrimaryTransferTime();
Review comment:
This could be replaced with `totalPrimaryTransferTime +=
details.getPrimaryTransferTime();`
##########
File path:
geode-management/src/main/java/org/apache/geode/management/internal/operation/RestoreRedundancyResultsImpl.java
##########
@@ -38,24 +37,55 @@
public static final String PRIMARY_TRANSFER_TIME = "Total primary transfer
time (ms) = ";
private static final long serialVersionUID = -1174735246756963521L;
- protected Map<String, RegionRedundancyStatus> zeroRedundancyRegions = new
HashMap<>();
- protected Map<String, RegionRedundancyStatus> underRedundancyRegions = new
HashMap<>();
- protected Map<String, RegionRedundancyStatus> satisfiedRedundancyRegions =
new HashMap<>();
+ protected Map<String, RegionRedundancyStatus> zeroRedundancyRegionsResults =
new HashMap<>();
+ protected Map<String, RegionRedundancyStatus> underRedundancyRegionsResults
= new HashMap<>();
+ protected Map<String, RegionRedundancyStatus>
satisfiedRedundancyRegionsResults = new HashMap<>();
protected int totalPrimaryTransfersCompleted;
- protected Duration totalPrimaryTransferTime = Duration.ZERO;
+ protected long totalPrimaryTransferTime = 0;
protected boolean success = true;
protected String statusMessage;
protected final List<String> includedRegionsWithNoMembers = new
ArrayList<>();
+ private RegionRedundancyStatus regionResult;
+
+ public void setZeroRedundancyRegionsResults(
+ Map<String, RegionRedundancyStatus> zeroRedundancyRegionsResults) {
+ this.zeroRedundancyRegionsResults = zeroRedundancyRegionsResults;
+ }
+
+ public void setUnderRedundancyRegionsResults(
+ Map<String, RegionRedundancyStatus> underRedundancyRegionsResults) {
+ this.underRedundancyRegionsResults = underRedundancyRegionsResults;
+ }
+
+ public void setSatisfiedRedundancyRegionsResults(
+ Map<String, RegionRedundancyStatus> satisfiedRedundancyRegionsResults) {
+ this.satisfiedRedundancyRegionsResults = satisfiedRedundancyRegionsResults;
+ }
+
+ public void setTotalPrimaryTransfersCompleted(int
totalPrimaryTransfersCompleted) {
+ this.totalPrimaryTransfersCompleted = totalPrimaryTransfersCompleted;
+ }
+
+ public void setTotalPrimaryTransferTime(long totalPrimaryTransferTime) {
+ this.totalPrimaryTransferTime = totalPrimaryTransferTime;
+ }
+
+ public void setRegionResult(RegionRedundancyStatus regionResult) {
+ this.regionResult = regionResult;
+ }
+
+
+ public RestoreRedundancyResultsImpl() {}
public void addRegionResults(RestoreRedundancyResults results) {
-
satisfiedRedundancyRegions.putAll(results.getSatisfiedRedundancyRegionResults());
- underRedundancyRegions.putAll(results.getUnderRedundancyRegionResults());
- zeroRedundancyRegions.putAll(results.getZeroRedundancyRegionResults());
+
satisfiedRedundancyRegionsResults.putAll(results.getSatisfiedRedundancyRegionResults());
+
underRedundancyRegionsResults.putAll(results.getUnderRedundancyRegionResults());
+
zeroRedundancyRegionsResults.putAll(results.getZeroRedundancyRegionResults());
totalPrimaryTransfersCompleted +=
results.getTotalPrimaryTransfersCompleted();
totalPrimaryTransferTime =
- totalPrimaryTransferTime.plus(results.getTotalPrimaryTransferTime());
+ totalPrimaryTransferTime + results.getTotalPrimaryTransferTime();
Review comment:
This can be replaced with `totalPrimaryTransferTime +=
results.getTotalPrimaryTransferTime();`
##########
File path:
geode-core/src/test/java/org/apache/geode/internal/cache/control/SerializableRestoreRedundancyResultsImplTest.java
##########
@@ -153,39 +157,64 @@ public void
addRegionResultsAddsToCorrectInternalMapAndAddsPrimaryReassignmentDe
when(regionResults.getSatisfiedRedundancyRegionResults())
.thenReturn(Collections.singletonMap(successfulRegionName,
successfulRegionResult));
when(regionResults.getTotalPrimaryTransfersCompleted()).thenReturn(transfersCompleted);
-
when(regionResults.getTotalPrimaryTransferTime()).thenReturn(Duration.ofMillis(transferTime));
+ when(regionResults.getTotalPrimaryTransferTime()).thenReturn(transferTime);
results.addRegionResults(regionResults);
Map<String, RegionRedundancyStatus> zeroRedundancyResults =
results.getZeroRedundancyRegionResults();
- assertThat(zeroRedundancyResults.size(), is(1));
- assertThat(zeroRedundancyResults.get(zeroRedundancyRegionName),
is(zeroRedundancyRegionResult));
+ assertThat(zeroRedundancyResults.size()).isEqualTo(1);
+ assertThat(zeroRedundancyResults.get(zeroRedundancyRegionName))
+ .isEqualTo(zeroRedundancyRegionResult);
Map<String, RegionRedundancyStatus> underRedundancyResults =
results.getUnderRedundancyRegionResults();
- assertThat(underRedundancyResults.size(), is(1));
- assertThat(underRedundancyResults.get(underRedundancyRegionName),
- is(underRedundancyRegionResult));
+ assertThat(underRedundancyResults.size()).isEqualTo(1);
+ assertThat(underRedundancyResults.get(underRedundancyRegionName))
+ .isEqualTo(underRedundancyRegionResult);
Map<String, RegionRedundancyStatus> successfulRegionResults =
results.getSatisfiedRedundancyRegionResults();
- assertThat(successfulRegionResults.size(), is(1));
- assertThat(successfulRegionResults.get(successfulRegionName),
is(successfulRegionResult));
+ assertThat(successfulRegionResults.size()).isEqualTo(1);
+
assertThat(successfulRegionResults.get(successfulRegionName)).isEqualTo(successfulRegionResult);
- assertThat(results.getTotalPrimaryTransfersCompleted(),
is(transfersCompleted));
- assertThat(results.getTotalPrimaryTransferTime().toMillis(),
is(transferTime));
+
assertThat(results.getTotalPrimaryTransfersCompleted()).isEqualTo(transfersCompleted);
+ assertThat(results.getTotalPrimaryTransferTime()).isEqualTo(transferTime);
}
@Test
public void addPrimaryDetailsUpdatesValue() {
- assertThat(results.getTotalPrimaryTransfersCompleted(), is(0));
- assertThat(results.getTotalPrimaryTransferTime().toMillis(), is(0L));
+ assertThat(results.getTotalPrimaryTransfersCompleted()).isEqualTo(0);
+ assertThat(results.getTotalPrimaryTransferTime()).isEqualTo(0L);
results.addPrimaryReassignmentDetails(details);
- assertThat(results.getTotalPrimaryTransfersCompleted(),
is(transfersCompleted));
- assertThat(results.getTotalPrimaryTransferTime().toMillis(),
is(transferTime));
+
assertThat(results.getTotalPrimaryTransfersCompleted()).isEqualTo(transfersCompleted);
+ assertThat(results.getTotalPrimaryTransferTime()).isEqualTo(transferTime);
results.addPrimaryReassignmentDetails(details);
- assertThat(results.getTotalPrimaryTransfersCompleted(),
is(transfersCompleted * 2));
- assertThat(results.getTotalPrimaryTransferTime().toMillis(),
is(transferTime * 2));
+
assertThat(results.getTotalPrimaryTransfersCompleted()).isEqualTo(transfersCompleted
* 2);
+ assertThat(results.getTotalPrimaryTransferTime()).isEqualTo(transferTime *
2);
+ }
+
+ @Test
+ public void testSerializable() throws JsonProcessingException {
Review comment:
This test sets multiple fields but does not verify all of them. Would it
be possible to confirm that all data in the object is correctly serialized and
deserialized, not just the message and status fields?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
