Skip to content

Commit

Permalink
feat: manage Notifier storage usage [DHIS2-17998] (2.41) (#19807)
Browse files Browse the repository at this point in the history
* feat: manage Notifier storage usage [DHIS2-17998] (#19738)

* feat: gist for overview lists, limit setting [DHIS2-17998]

* refactor: NotifierStore implemented

* feat: clear and cap API and endpoints

* feat: cap for redis store

* fix: cap and clean consistency, log level filter for scheduling only

* chore: API cleanup, javadoc, some test fixes

* fix: notifier tests

* fix: hide transient empty in-memory stores in the API

* fix: update test assert with new message

* fix: remove notifier stubbing from mock test

* fix: e2e test assert for updated message

* fix: mock test setup, dependencies

* fix: add jackson core back in

* fix: exclude jackson core from dependency check

* fix: sonar issues

* test: notifier API tests for in-memory and redis

* fix: sonar warnings

* chore: fix typo

* chore: prevent null message logging + log cleanup [DHIS2-17998] (#19800)

* fix: maven dependencies + mock tests

* fix: add Long conversion

* fix: e2e test message expectations after change in wording
  • Loading branch information
jbee authored Jan 30, 2025
1 parent 5f9ce8b commit c081837
Show file tree
Hide file tree
Showing 37 changed files with 2,418 additions and 1,307 deletions.
6 changes: 3 additions & 3 deletions dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/UID.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,14 @@
@Getter
@EqualsAndHashCode
public final class UID implements Serializable {
private static final String VALID_UID_FORMAT =
"UID must be an alphanumeric string of 11 characters starting with a letter.";

private final String value;

private UID(String value) {
if (!CodeGenerator.isValidUid(value)) {
throw new IllegalArgumentException(VALID_UID_FORMAT);
throw new IllegalArgumentException(
"UID must be an alphanumeric string of 11 characters starting with a letter, but was: "
+ value);
}
this.value = value;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ public boolean isDefaultExecutedByCreator() {
}

/**
* @implNote since 2.42 all jobs forward to the {@code Notifier} but those not included here use
* {@link org.hisp.dhis.system.notification.NotificationLevel#ERROR}.
* @return true, if {@link JobProgress} events should be forwarded to the {@link
* org.eclipse.emf.common.notify.Notifier} API, otherwise false
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import org.hisp.dhis.scheduling.JobConfiguration;
import org.hisp.dhis.security.LoginPageLayout;
import org.hisp.dhis.sms.config.SmsConfiguration;
import org.hisp.dhis.system.notification.NotificationLevel;

/**
* @author Lars Helge Overland
Expand Down Expand Up @@ -295,7 +296,13 @@ public enum SettingKey {
* the app does not exist *
*/
GLOBAL_SHELL_APP_NAME("globalShellAppName", "global-app-shell", String.class, false, false),
;

NOTIFIER_LOG_LEVEL("notifierLogLevel", NotificationLevel.DEBUG, NotificationLevel.class),
NOTIFIER_MAX_MESSAGES_PER_JOB("notifierMaxMessagesPerJob", 500, Integer.class),
NOTIFIER_MAX_AGE_DAYS("notifierMaxAgeDays", 7, Integer.class),
NOTIFIER_MAX_JOBS_PER_TYPE("notifierMaxJobsPerType", 500, Integer.class),
NOTIFIER_GIST_OVERVIEW("notifierGistOverview", true, Boolean.class),
NOTIFIER_CLEAN_AFTER_IDLE_TIME("notifierCleanAfterIdleTime", 60_000L, Long.class);

private final String name;

Expand Down Expand Up @@ -363,6 +370,8 @@ public static Serializable getAsRealClass(String name, String value) {
return Double.valueOf(value);
} else if (Integer.class.isAssignableFrom(settingClazz)) {
return Integer.valueOf(value);
} else if (Long.class.isAssignableFrom(settingClazz)) {
return Long.valueOf(value);
} else if (Boolean.class.isAssignableFrom(settingClazz)) {
return Boolean.valueOf(value);
} else if (Locale.class.isAssignableFrom(settingClazz)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright (c) 2004-2025, University of Oslo
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
* Redistributions of source code must retain the above copyright notice, this
* list of conditions and the following disclaimer.
*
* Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
* Neither the name of the HISP project nor the names of its contributors may
* be used to endorse or promote products derived from this software without
* specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
package org.hisp.dhis.setting;

import java.io.Serializable;

public interface SystemSettingsProvider {

<T extends Serializable> T getSystemSetting(SettingKey key, Class<T> type);

/**
* Returns the system setting value for the given key. If no value exists, returns the default
* value as defined by the given default value.
*
* @param key the system setting key.
* @return the setting value.
*/
<T extends Serializable> T getSystemSetting(SettingKey key, T defaultValue);

String getStringSetting(SettingKey key);

Integer getIntegerSetting(SettingKey key);

int getIntSetting(SettingKey key);

Boolean getBooleanSetting(SettingKey key);

boolean getBoolSetting(SettingKey key);
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.hisp.dhis.message.MessageService;
import org.hisp.dhis.setting.SettingKey;
import org.hisp.dhis.setting.SystemSettingManager;
import org.hisp.dhis.system.notification.NotificationLevel;
import org.hisp.dhis.system.notification.Notifier;
import org.hisp.dhis.user.AuthenticationService;
import org.hisp.dhis.user.UserDetails;
Expand Down Expand Up @@ -258,10 +259,12 @@ private static void logError(String message, Exception ex) {
}

private JobProgress startRecording(@Nonnull JobConfiguration job, @Nonnull Runnable observer) {
JobProgress tracker =
NotificationLevel level =
job.getJobType().isUsingNotifications()
? new NotifierJobProgress(notifier, job)
: NoopJobProgress.INSTANCE;
? systemSettings.getSystemSetting(
SettingKey.NOTIFIER_LOG_LEVEL, NotificationLevel.DEBUG)
: NotificationLevel.ERROR;
JobProgress tracker = new NotifierJobProgress(notifier, job, level);
boolean logInfoOnDebug =
job.getSchedulingType() != SchedulingType.ONCE_ASAP
&& job.getLastExecuted() != null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@

import static org.apache.commons.lang3.StringUtils.isNotEmpty;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.concurrent.atomic.AtomicBoolean;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import lombok.RequiredArgsConstructor;
import org.hisp.dhis.jsontree.JsonValue;
import org.hisp.dhis.system.notification.NotificationDataType;
import org.hisp.dhis.system.notification.NotificationLevel;
import org.hisp.dhis.system.notification.Notifier;
Expand All @@ -45,16 +47,27 @@
*/
@RequiredArgsConstructor
public class NotifierJobProgress implements JobProgress {
private final Notifier notifier;

private final Notifier notifier;
private final JobConfiguration jobId;

private final NotificationLevel level;
private final AtomicBoolean hasCleared = new AtomicBoolean();

private int stageItems;

private int stageItem;

private boolean isLoggedLoop() {
return NotificationLevel.LOOP.ordinal() >= level.ordinal();
}

private boolean isLoggedInfo() {
return NotificationLevel.INFO.ordinal() >= level.ordinal();
}

private boolean isLoggedError() {
return NotificationLevel.ERROR.ordinal() >= level.ordinal();
}

@Override
public void startingProcess(String description, Object... args) {
String message =
Expand All @@ -64,6 +77,7 @@ public void startingProcess(String description, Object... args) {
if (hasCleared.compareAndSet(false, true)) {
notifier.clear(jobId);
}
// Note: intentionally no log level check - always log first
notifier.notify(
jobId,
NotificationLevel.INFO,
Expand All @@ -75,40 +89,43 @@ public void startingProcess(String description, Object... args) {

@Override
public void completedProcess(String summary, Object... args) {
// Note: intentionally no log level check - always log last
notifier.notify(jobId, format(summary, args), true);
}

@Override
public void failedProcess(String error, Object... args) {
public void failedProcess(@CheckForNull String error, Object... args) {
// Note: intentionally no log level check - always log last
notifier.notify(jobId, NotificationLevel.ERROR, format(error, args), true);
}

@Override
public void startingStage(String description, int workItems, FailurePolicy onFailure) {
public void startingStage(
@Nonnull String description, int workItems, @Nonnull FailurePolicy onFailure) {
stageItems = workItems;
stageItem = 0;
if (isNotEmpty(description)) {
if (isLoggedInfo() && isNotEmpty(description)) {
notifier.notify(jobId, description);
}
}

@Override
public void completedStage(String summary, Object... args) {
if (isNotEmpty(summary)) {
if (isLoggedInfo() && isNotEmpty(summary)) {
notifier.notify(jobId, format(summary, args));
}
}

@Override
public void failedStage(String error, Object... args) {
if (isNotEmpty(error)) {
public void failedStage(@Nonnull String error, Object... args) {
if (isLoggedError() && isNotEmpty(error)) {
notifier.notify(jobId, NotificationLevel.ERROR, format(error, args), false);
}
}

@Override
public void startingWorkItem(String description, FailurePolicy onFailure) {
if (isNotEmpty(description)) {
public void startingWorkItem(@Nonnull String description, @Nonnull FailurePolicy onFailure) {
if (isLoggedLoop() && isNotEmpty(description)) {
String nOf = "[" + (stageItems > 0 ? stageItem + "/" + stageItems : "" + stageItem) + "] ";
notifier.notify(jobId, NotificationLevel.LOOP, nOf + description, false);
}
Expand All @@ -117,26 +134,26 @@ public void startingWorkItem(String description, FailurePolicy onFailure) {

@Override
public void completedWorkItem(String summary, Object... args) {
if (isNotEmpty(summary)) {
if (isLoggedLoop() && isNotEmpty(summary)) {
String nOf = "[" + (stageItems > 0 ? stageItem + "/" + stageItems : "" + stageItem) + "] ";
notifier.notify(jobId, NotificationLevel.LOOP, nOf + format(summary, args), false);
}
}

@Override
public void failedWorkItem(String error, Object... args) {
if (isNotEmpty(error)) {
public void failedWorkItem(@Nonnull String error, Object... args) {
if (isLoggedError() && isNotEmpty(error)) {
notifier.notify(jobId, NotificationLevel.ERROR, format(error, args), false);
}
}

private JsonNode getJobParameterData() {
private JsonValue getJobParameterData() {
JobParameters params = jobId.getJobParameters();
if (params == null) {
return null;
}
try {
return new ObjectMapper().valueToTree(params);
return JsonValue.of(new ObjectMapper().writeValueAsString(params));
} catch (Exception ex) {
return null;
}
Expand Down
Loading

0 comments on commit c081837

Please sign in to comment.