Skip to content

Commit da608aa

Browse files
kaituokaituo
authored
Code Refactoring for CommonMessages (#902)
* Code Refactoring for CommonMessages In this pull request, I have refactored the code related to shared log or error messages in both AD and forecasting modules to CommonMessages. Additionally, the previously used CommonErrorMessages has been renamed to ADCommonMessages. For the Forecasting module, I have introduced new names in ForecastCommonMessages. Testing done: * gradle build Signed-off-by: Kaituo Li <[email protected]> * Update string constants wording Signed-off-by: Kaituo Li <[email protected]> * improve wording and remove redundant variables Signed-off-by: Kaituo Li <[email protected]> --------- Signed-off-by: Kaituo Li <[email protected]>
1 parent 041d6ce commit da608aa

File tree

94 files changed

+495
-420
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

94 files changed

+495
-420
lines changed

‎src/main/java/org/opensearch/ad/AnomalyDetectorProfileRunner.java

+12-12
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@
1111

1212
package org.opensearch.ad;
1313

14-
import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_FIND_DETECTOR_MSG;
15-
import static org.opensearch.ad.constant.CommonErrorMessages.FAIL_TO_PARSE_DETECTOR_MSG;
14+
import static org.opensearch.ad.constant.ADCommonMessages.FAIL_TO_PARSE_DETECTOR_MSG;
1615
import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
1716
import static org.opensearch.rest.RestStatus.BAD_REQUEST;
1817
import static org.opensearch.rest.RestStatus.INTERNAL_SERVER_ERROR;
18+
import static org.opensearch.timeseries.constant.CommonMessages.FAIL_TO_FIND_CONFIG_MSG;
1919

2020
import java.util.List;
2121
import java.util.Map;
@@ -33,8 +33,8 @@
3333
import org.opensearch.action.search.SearchResponse;
3434
import org.opensearch.ad.common.exception.NotSerializedADExceptionName;
3535
import org.opensearch.ad.common.exception.ResourceNotFoundException;
36+
import org.opensearch.ad.constant.ADCommonMessages;
3637
import org.opensearch.ad.constant.ADCommonName;
37-
import org.opensearch.ad.constant.CommonErrorMessages;
3838
import org.opensearch.ad.model.ADTaskType;
3939
import org.opensearch.ad.model.AnomalyDetector;
4040
import org.opensearch.ad.model.AnomalyDetectorJob;
@@ -109,7 +109,7 @@ public AnomalyDetectorProfileRunner(
109109

110110
public void profile(String detectorId, ActionListener<DetectorProfile> listener, Set<DetectorProfileName> profilesToCollect) {
111111
if (profilesToCollect.isEmpty()) {
112-
listener.onFailure(new IllegalArgumentException(CommonErrorMessages.EMPTY_PROFILES_COLLECT));
112+
listener.onFailure(new IllegalArgumentException(ADCommonMessages.EMPTY_PROFILES_COLLECT));
113113
return;
114114
}
115115
calculateTotalResponsesToWait(detectorId, profilesToCollect, listener);
@@ -136,11 +136,11 @@ private void calculateTotalResponsesToWait(
136136
listener.onFailure(new OpenSearchStatusException(FAIL_TO_PARSE_DETECTOR_MSG + detectorId, BAD_REQUEST));
137137
}
138138
} else {
139-
listener.onFailure(new OpenSearchStatusException(FAIL_TO_FIND_DETECTOR_MSG + detectorId, BAD_REQUEST));
139+
listener.onFailure(new OpenSearchStatusException(FAIL_TO_FIND_CONFIG_MSG + detectorId, BAD_REQUEST));
140140
}
141141
}, exception -> {
142-
logger.error(FAIL_TO_FIND_DETECTOR_MSG + detectorId, exception);
143-
listener.onFailure(new OpenSearchStatusException(FAIL_TO_FIND_DETECTOR_MSG + detectorId, INTERNAL_SERVER_ERROR));
142+
logger.error(FAIL_TO_FIND_CONFIG_MSG + detectorId, exception);
143+
listener.onFailure(new OpenSearchStatusException(FAIL_TO_FIND_CONFIG_MSG + detectorId, INTERNAL_SERVER_ERROR));
144144
}));
145145
}
146146

@@ -207,7 +207,7 @@ private void prepareProfile(
207207
new MultiResponsesDelegateActionListener<DetectorProfile>(
208208
listener,
209209
totalResponsesToWait,
210-
CommonErrorMessages.FAIL_FETCH_ERR_MSG + detectorId,
210+
ADCommonMessages.FAIL_FETCH_ERR_MSG + detectorId,
211211
false
212212
);
213213
if (profilesToCollect.contains(DetectorProfileName.ERROR)) {
@@ -266,7 +266,7 @@ private void prepareProfile(
266266
}
267267

268268
} catch (Exception e) {
269-
logger.error(CommonErrorMessages.FAIL_TO_GET_PROFILE_MSG, e);
269+
logger.error(ADCommonMessages.FAIL_TO_GET_PROFILE_MSG, e);
270270
listener.onFailure(e);
271271
}
272272
} else {
@@ -277,7 +277,7 @@ private void prepareProfile(
277277
logger.info(exception.getMessage());
278278
onGetDetectorForPrepare(detectorId, listener, profilesToCollect);
279279
} else {
280-
logger.error(CommonErrorMessages.FAIL_TO_GET_PROFILE_MSG + detectorId);
280+
logger.error(ADCommonMessages.FAIL_TO_GET_PROFILE_MSG + detectorId);
281281
listener.onFailure(exception);
282282
}
283283
}));
@@ -304,7 +304,7 @@ private void profileEntityStats(MultiResponsesDelegateActionListener<DetectorPro
304304
DetectorProfile profile = profileBuilder.totalEntities(value).build();
305305
listener.onResponse(profile);
306306
}, searchException -> {
307-
logger.warn(CommonErrorMessages.FAIL_TO_GET_TOTAL_ENTITIES + detector.getDetectorId());
307+
logger.warn(ADCommonMessages.FAIL_TO_GET_TOTAL_ENTITIES + detector.getDetectorId());
308308
listener.onFailure(searchException);
309309
});
310310
// using the original context in listener as user roles have no permissions for internal operations like fetching a
@@ -353,7 +353,7 @@ private void profileEntityStats(MultiResponsesDelegateActionListener<DetectorPro
353353
DetectorProfile profile = profileBuilder.totalEntities(Long.valueOf(compositeAgg.getBuckets().size())).build();
354354
listener.onResponse(profile);
355355
}, searchException -> {
356-
logger.warn(CommonErrorMessages.FAIL_TO_GET_TOTAL_ENTITIES + detector.getDetectorId());
356+
logger.warn(ADCommonMessages.FAIL_TO_GET_TOTAL_ENTITIES + detector.getDetectorId());
357357
listener.onFailure(searchException);
358358
});
359359
// using the original context in listener as user roles have no permissions for internal operations like fetching a

‎src/main/java/org/opensearch/ad/EntityProfileRunner.java

+8-8
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525
import org.opensearch.action.get.GetRequest;
2626
import org.opensearch.action.search.SearchRequest;
2727
import org.opensearch.action.search.SearchResponse;
28+
import org.opensearch.ad.constant.ADCommonMessages;
2829
import org.opensearch.ad.constant.ADCommonName;
29-
import org.opensearch.ad.constant.CommonErrorMessages;
3030
import org.opensearch.ad.model.AnomalyDetector;
3131
import org.opensearch.ad.model.AnomalyDetectorJob;
3232
import org.opensearch.ad.model.AnomalyResult;
@@ -56,6 +56,7 @@
5656
import org.opensearch.index.query.TermQueryBuilder;
5757
import org.opensearch.search.aggregations.AggregationBuilders;
5858
import org.opensearch.search.builder.SearchSourceBuilder;
59+
import org.opensearch.timeseries.constant.CommonMessages;
5960
import org.opensearch.timeseries.constant.CommonName;
6061

6162
public class EntityProfileRunner extends AbstractProfileRunner {
@@ -90,7 +91,7 @@ public void profile(
9091
ActionListener<EntityProfile> listener
9192
) {
9293
if (profilesToCollect == null || profilesToCollect.size() == 0) {
93-
listener.onFailure(new IllegalArgumentException(CommonErrorMessages.EMPTY_PROFILES_COLLECT));
94+
listener.onFailure(new IllegalArgumentException(ADCommonMessages.EMPTY_PROFILES_COLLECT));
9495
return;
9596
}
9697
GetRequest getDetectorRequest = new GetRequest(CommonName.CONFIG_INDEX, detectorId);
@@ -109,16 +110,15 @@ public void profile(
109110
if (categoryFields == null || categoryFields.size() == 0) {
110111
listener.onFailure(new IllegalArgumentException(NOT_HC_DETECTOR_ERR_MSG));
111112
} else if (categoryFields.size() > maxCategoryFields) {
112-
listener
113-
.onFailure(new IllegalArgumentException(CommonErrorMessages.getTooManyCategoricalFieldErr(maxCategoryFields)));
113+
listener.onFailure(new IllegalArgumentException(CommonMessages.getTooManyCategoricalFieldErr(maxCategoryFields)));
114114
} else {
115115
validateEntity(entityValue, categoryFields, detectorId, profilesToCollect, detector, listener);
116116
}
117117
} catch (Exception t) {
118118
listener.onFailure(t);
119119
}
120120
} else {
121-
listener.onFailure(new IllegalArgumentException(CommonErrorMessages.FAIL_TO_FIND_DETECTOR_MSG + detectorId));
121+
listener.onFailure(new IllegalArgumentException(CommonMessages.FAIL_TO_FIND_CONFIG_MSG + detectorId));
122122
}
123123
}, listener::onFailure));
124124
}
@@ -245,7 +245,7 @@ private void getJob(
245245
new MultiResponsesDelegateActionListener<EntityProfile>(
246246
listener,
247247
totalResponsesToWait,
248-
CommonErrorMessages.FAIL_FETCH_ERR_MSG + entityValue + " of detector " + detectorId,
248+
ADCommonMessages.FAIL_FETCH_ERR_MSG + entityValue + " of detector " + detectorId,
249249
false
250250
);
251251

@@ -308,7 +308,7 @@ private void getJob(
308308
}));
309309
}
310310
} catch (Exception e) {
311-
logger.error(CommonErrorMessages.FAIL_TO_GET_PROFILE_MSG, e);
311+
logger.error(ADCommonMessages.FAIL_TO_GET_PROFILE_MSG, e);
312312
listener.onFailure(e);
313313
}
314314
} else {
@@ -319,7 +319,7 @@ private void getJob(
319319
logger.info(exception.getMessage());
320320
sendUnknownState(profilesToCollect, entityValue, true, listener);
321321
} else {
322-
logger.error(CommonErrorMessages.FAIL_TO_GET_PROFILE_MSG + detectorId, exception);
322+
logger.error(ADCommonMessages.FAIL_TO_GET_PROFILE_MSG + detectorId, exception);
323323
listener.onFailure(exception);
324324
}
325325
}));

‎src/main/java/org/opensearch/ad/ExecuteADResultResponseRecorder.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
package org.opensearch.ad;
1313

14-
import static org.opensearch.ad.constant.CommonErrorMessages.CAN_NOT_FIND_LATEST_TASK;
14+
import static org.opensearch.ad.constant.ADCommonMessages.CAN_NOT_FIND_LATEST_TASK;
1515

1616
import java.time.Instant;
1717
import java.util.ArrayList;
@@ -26,7 +26,7 @@
2626
import org.opensearch.ad.common.exception.AnomalyDetectionException;
2727
import org.opensearch.ad.common.exception.EndRunException;
2828
import org.opensearch.ad.common.exception.ResourceNotFoundException;
29-
import org.opensearch.ad.constant.CommonErrorMessages;
29+
import org.opensearch.ad.constant.ADCommonMessages;
3030
import org.opensearch.ad.indices.ADIndex;
3131
import org.opensearch.ad.indices.AnomalyDetectionIndices;
3232
import org.opensearch.ad.model.AnomalyDetector;
@@ -307,7 +307,7 @@ public void indexAnomalyResultException(
307307
anomalyResultHandler.index(anomalyResult, detectorId, resultIndex);
308308
}
309309

310-
if (errorMessage.contains(CommonErrorMessages.NO_MODEL_ERR_MSG) && !detector.isMultiCategoryDetector()) {
310+
if (errorMessage.contains(ADCommonMessages.NO_MODEL_ERR_MSG) && !detector.isMultiCategoryDetector()) {
311311
// single stream detector raises ResourceNotFoundException containing CommonErrorMessages.NO_CHECKPOINT_ERR_MSG
312312
// when there is no checkpoint.
313313
// Delay real time cache update by one minute so we will have trained models by then and update the state

‎src/main/java/org/opensearch/ad/NodeStateManager.java

+2-5
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import org.opensearch.action.get.GetResponse;
3333
import org.opensearch.ad.common.exception.EndRunException;
3434
import org.opensearch.ad.constant.ADCommonName;
35-
import org.opensearch.ad.constant.CommonErrorMessages;
3635
import org.opensearch.ad.ml.SingleStreamModelIdMapper;
3736
import org.opensearch.ad.model.AnomalyDetector;
3837
import org.opensearch.ad.model.AnomalyDetectorJob;
@@ -48,6 +47,7 @@
4847
import org.opensearch.common.xcontent.XContentType;
4948
import org.opensearch.core.xcontent.NamedXContentRegistry;
5049
import org.opensearch.core.xcontent.XContentParser;
50+
import org.opensearch.timeseries.constant.CommonMessages;
5151
import org.opensearch.timeseries.constant.CommonName;
5252

5353
/**
@@ -153,10 +153,7 @@ private ActionListener<GetResponse> onGetDetectorResponse(String adID, ActionLis
153153
AnomalyDetector detector = AnomalyDetector.parse(parser, response.getId());
154154
// end execution if all features are disabled
155155
if (detector.getEnabledFeatureIds().isEmpty()) {
156-
listener
157-
.onFailure(
158-
new EndRunException(adID, CommonErrorMessages.ALL_FEATURES_DISABLED_ERR_MSG, true).countedInStats(false)
159-
);
156+
listener.onFailure(new EndRunException(adID, CommonMessages.ALL_FEATURES_DISABLED_ERR_MSG, true).countedInStats(false));
160157
return;
161158
}
162159
NodeState state = states.computeIfAbsent(adID, id -> new NodeState(id, clock));

‎src/main/java/org/opensearch/ad/caching/PriorityCache.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
import org.opensearch.ad.MemoryTracker.Origin;
4545
import org.opensearch.ad.common.exception.AnomalyDetectionException;
4646
import org.opensearch.ad.common.exception.LimitExceededException;
47-
import org.opensearch.ad.constant.CommonErrorMessages;
4847
import org.opensearch.ad.ml.CheckpointDao;
4948
import org.opensearch.ad.ml.EntityModel;
5049
import org.opensearch.ad.ml.ModelManager.ModelType;
@@ -63,6 +62,7 @@
6362
import org.opensearch.common.settings.Settings;
6463
import org.opensearch.common.unit.TimeValue;
6564
import org.opensearch.threadpool.ThreadPool;
65+
import org.opensearch.timeseries.constant.CommonMessages;
6666

6767
import com.google.common.cache.Cache;
6868
import com.google.common.cache.CacheBuilder;
@@ -482,7 +482,7 @@ private CacheBuffer computeBufferIfAbsent(AnomalyDetector detector, String detec
482482
// Put tryClearUpMemory after consumeMemory to prevent that.
483483
tryClearUpMemory();
484484
} else {
485-
throw new LimitExceededException(detectorId, CommonErrorMessages.MEMORY_LIMIT_EXCEEDED_ERR_MSG);
485+
throw new LimitExceededException(detectorId, CommonMessages.MEMORY_LIMIT_EXCEEDED_ERR_MSG);
486486
}
487487

488488
}

0 commit comments

Comments
 (0)