Skip to content

Commit 10f5ded

Browse files
enricocolasanteenricocolasante
authored
fix: Validate PRV based on type [DHIS2-20787] (#23663) (#23738)
1 parent dc28d91 commit 10f5ded

4 files changed

Lines changed: 232 additions & 15 deletions

File tree

‎dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,13 @@ public enum ErrorCode {
262262
E4053("Program stage `{0}` must reference a program"),
263263
E4057("The Program Rule name {0} already exist in Program {1}"),
264264

265+
E4058("Program Rule `{0}` with Action Type `{1}` has irrelevant reference objects"),
266+
E4059("ProgramRuleVariable `{0}` with source type `{1}` requires a data element"),
267+
E4089("ProgramRuleVariable `{0}` with source type `{1}` requires a tracked entity attribute"),
268+
E4090("ProgramRuleVariable `{0}` is missing a source type"),
269+
E4091("ProgramRuleVariable `{0}` with source type `{1}` requires a program stage"),
270+
E4092("ProgramRuleVariable `{0}` with source type `{1}` requires a value type"),
271+
265272
/* Metadata Validation (continued) */
266273
E4060("Object could not be deleted: {0}"),
267274
E4061("DashboardItem `{0}` object reference `{1}` with id `{2}` not found or not accessible"),

‎dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/ProgramRuleVariableObjectBundleHook.java‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ public void validate(
9595
Consumer<ErrorReport> addReports) {
9696
validateUniqueProgramRuleName(bundle, programRuleVariable, addReports);
9797
validateProgramRuleNameKeyWords(programRuleVariable, addReports);
98+
validateSourceTypeReference(programRuleVariable, addReports);
9899
}
99100

100101
private void validateUniqueProgramRuleName(
@@ -163,6 +164,53 @@ private void validateProgramRuleNameKeyWords(
163164
}
164165
}
165166

167+
private void validateSourceTypeReference(
168+
ProgramRuleVariable programRuleVariable, Consumer<ErrorReport> addReports) {
169+
ProgramRuleVariableSourceType sourceType = programRuleVariable.getSourceType();
170+
if (sourceType == null) {
171+
addReports.accept(
172+
new ErrorReport(
173+
ProgramRuleVariable.class, ErrorCode.E4090, programRuleVariable.getName()));
174+
return;
175+
}
176+
if (ProgramRuleVariableSourceType.getDataTypes().contains(sourceType)
177+
&& programRuleVariable.getDataElement() == null) {
178+
addReports.accept(
179+
new ErrorReport(
180+
ProgramRuleVariable.class,
181+
ErrorCode.E4059,
182+
programRuleVariable.getName(),
183+
sourceType));
184+
}
185+
if (sourceType == ProgramRuleVariableSourceType.DATAELEMENT_NEWEST_EVENT_PROGRAM_STAGE
186+
&& programRuleVariable.getProgramStage() == null) {
187+
addReports.accept(
188+
new ErrorReport(
189+
ProgramRuleVariable.class,
190+
ErrorCode.E4091,
191+
programRuleVariable.getName(),
192+
sourceType));
193+
}
194+
if (ProgramRuleVariableSourceType.getAttributeTypes().contains(sourceType)
195+
&& programRuleVariable.getAttribute() == null) {
196+
addReports.accept(
197+
new ErrorReport(
198+
ProgramRuleVariable.class,
199+
ErrorCode.E4089,
200+
programRuleVariable.getName(),
201+
sourceType));
202+
}
203+
if (sourceType == ProgramRuleVariableSourceType.CALCULATED_VALUE
204+
&& programRuleVariable.getValueType() == null) {
205+
addReports.accept(
206+
new ErrorReport(
207+
ProgramRuleVariable.class,
208+
ErrorCode.E4092,
209+
programRuleVariable.getName(),
210+
sourceType));
211+
}
212+
}
213+
166214
@Override
167215
public void preUpdate(
168216
ProgramRuleVariable variable, ProgramRuleVariable persistedObject, ObjectBundle bundle) {

‎dhis-2/dhis-services/dhis-service-dxf2/src/test/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/ProgramRuleVariableObjectBundleHookTest.java‎

Lines changed: 175 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,24 +30,38 @@
3030
import static org.hisp.dhis.dxf2.Constants.PROGRAM_RULE_VARIABLE_NAME_INVALID_KEYWORDS;
3131
import static org.hisp.dhis.feedback.ErrorCode.E4051;
3232
import static org.hisp.dhis.feedback.ErrorCode.E4052;
33+
import static org.hisp.dhis.feedback.ErrorCode.E4059;
34+
import static org.hisp.dhis.feedback.ErrorCode.E4089;
35+
import static org.hisp.dhis.feedback.ErrorCode.E4090;
36+
import static org.hisp.dhis.feedback.ErrorCode.E4091;
37+
import static org.hisp.dhis.feedback.ErrorCode.E4092;
3338
import static org.junit.jupiter.api.Assertions.assertEquals;
3439
import static org.junit.jupiter.api.Assertions.assertTrue;
3540
import static org.mockito.ArgumentMatchers.anyString;
41+
import static org.mockito.Mockito.lenient;
3642
import static org.mockito.Mockito.when;
3743

3844
import java.util.Collections;
3945
import java.util.List;
46+
import java.util.stream.Stream;
4047
import org.hibernate.Session;
4148
import org.hibernate.SessionFactory;
4249
import org.hibernate.query.Query;
50+
import org.hisp.dhis.common.ValueType;
51+
import org.hisp.dhis.dataelement.DataElement;
4352
import org.hisp.dhis.dxf2.metadata.objectbundle.ObjectBundle;
4453
import org.hisp.dhis.feedback.ErrorReport;
4554
import org.hisp.dhis.importexport.ImportStrategy;
4655
import org.hisp.dhis.program.Program;
56+
import org.hisp.dhis.program.ProgramStage;
4757
import org.hisp.dhis.programrule.ProgramRuleVariable;
58+
import org.hisp.dhis.programrule.ProgramRuleVariableSourceType;
59+
import org.hisp.dhis.trackedentity.TrackedEntityAttribute;
4860
import org.junit.jupiter.api.BeforeEach;
4961
import org.junit.jupiter.api.Test;
5062
import org.junit.jupiter.api.extension.ExtendWith;
63+
import org.junit.jupiter.params.ParameterizedTest;
64+
import org.junit.jupiter.params.provider.MethodSource;
5165
import org.mockito.ArgumentCaptor;
5266
import org.mockito.Captor;
5367
import org.mockito.InjectMocks;
@@ -81,32 +95,40 @@ public void setUp() {
8195
when(sessionFactory.getCurrentSession()).thenReturn(session);
8296
when(session.createQuery(anyString(), classArgumentCaptor.capture())).thenReturn(query);
8397
when(program.getUid()).thenReturn("uid");
98+
// Common defaults; individual tests override what they need.
99+
lenient().when(programRuleVariable.getProgram()).thenReturn(program);
100+
lenient().when(programRuleVariable.getName()).thenReturn("someVar");
101+
lenient().when(objectBundle.getImportMode()).thenReturn(ImportStrategy.CREATE);
102+
lenient().when(query.getResultList()).thenReturn(Collections.emptyList());
84103
}
85104

86105
@Test
87106
void shouldFailInsertAlreadyExisting() {
88-
when(programRuleVariable.getProgram()).thenReturn(program);
89-
when(objectBundle.getImportMode()).thenReturn(ImportStrategy.CREATE);
90107
when(query.getResultList()).thenReturn(Collections.singletonList(new ProgramRuleVariable()));
91-
92108
when(programRuleVariable.getName()).thenReturn("word");
109+
when(programRuleVariable.getSourceType())
110+
.thenReturn(ProgramRuleVariableSourceType.CALCULATED_VALUE);
111+
when(programRuleVariable.getValueType()).thenReturn(ValueType.TEXT);
112+
93113
List<ErrorReport> errorReports =
94114
programRuleVariableObjectBundleHook.validate(programRuleVariable, objectBundle);
115+
95116
assertEquals(1, errorReports.size());
96117
assertTrue(errorReports.stream().anyMatch(e -> e.getErrorCode().equals(E4051)));
97118
}
98119

99120
@Test
100121
void shouldNotFailUpdateExistingSameUid() {
101-
when(programRuleVariable.getProgram()).thenReturn(program);
102122
when(objectBundle.getImportMode()).thenReturn(ImportStrategy.CREATE_AND_UPDATE);
123+
when(programRuleVariable.getSourceType())
124+
.thenReturn(ProgramRuleVariableSourceType.CALCULATED_VALUE);
125+
when(programRuleVariable.getValueType()).thenReturn(ValueType.TEXT);
103126

104127
ProgramRuleVariable existingProgramRuleVariable = new ProgramRuleVariable();
105128
existingProgramRuleVariable.setName("word");
106129
existingProgramRuleVariable.setUid("uid1");
107130

108131
when(query.getResultList()).thenReturn(Collections.singletonList(existingProgramRuleVariable));
109-
110132
when(programRuleVariable.getName()).thenReturn("word");
111133
when(programRuleVariable.getUid()).thenReturn("uid1");
112134

@@ -118,8 +140,10 @@ void shouldNotFailUpdateExistingSameUid() {
118140

119141
@Test
120142
void shouldNotFailUpdateExistingMoreThanOneSameUid() {
121-
when(programRuleVariable.getProgram()).thenReturn(program);
122143
when(objectBundle.getImportMode()).thenReturn(ImportStrategy.CREATE_AND_UPDATE);
144+
when(programRuleVariable.getSourceType())
145+
.thenReturn(ProgramRuleVariableSourceType.CALCULATED_VALUE);
146+
when(programRuleVariable.getValueType()).thenReturn(ValueType.TEXT);
123147

124148
ProgramRuleVariable existingProgramRuleVariable = new ProgramRuleVariable();
125149
existingProgramRuleVariable.setName("word");
@@ -131,7 +155,6 @@ void shouldNotFailUpdateExistingMoreThanOneSameUid() {
131155

132156
when(query.getResultList())
133157
.thenReturn(List.of(existingProgramRuleVariable, anotherExistingProgramRuleVariable));
134-
135158
when(programRuleVariable.getName()).thenReturn("word");
136159
when(programRuleVariable.getUid()).thenReturn("uid1");
137160

@@ -143,15 +166,16 @@ void shouldNotFailUpdateExistingMoreThanOneSameUid() {
143166

144167
@Test
145168
void shouldFailUpdateExistingDifferentUid() {
146-
when(programRuleVariable.getProgram()).thenReturn(program);
147169
when(objectBundle.getImportMode()).thenReturn(ImportStrategy.CREATE_AND_UPDATE);
170+
when(programRuleVariable.getSourceType())
171+
.thenReturn(ProgramRuleVariableSourceType.CALCULATED_VALUE);
172+
when(programRuleVariable.getValueType()).thenReturn(ValueType.TEXT);
148173

149174
ProgramRuleVariable existingProgramRuleVariable = new ProgramRuleVariable();
150175
existingProgramRuleVariable.setName("word");
151176
existingProgramRuleVariable.setUid("uid1");
152177

153178
when(query.getResultList()).thenReturn(Collections.singletonList(existingProgramRuleVariable));
154-
155179
when(programRuleVariable.getName()).thenReturn("word");
156180
when(programRuleVariable.getUid()).thenReturn("uid2");
157181

@@ -164,11 +188,12 @@ void shouldFailUpdateExistingDifferentUid() {
164188

165189
@Test
166190
void shouldFailValidationInvalidCountAndInvalidName() {
167-
when(programRuleVariable.getProgram()).thenReturn(program);
168-
when(objectBundle.getImportMode()).thenReturn(ImportStrategy.CREATE);
169191
when(query.getResultList()).thenReturn(Collections.singletonList(new ProgramRuleVariable()));
170192
when(programRuleVariable.getName())
171193
.thenReturn("Word " + PROGRAM_RULE_VARIABLE_NAME_INVALID_KEYWORDS.get(0) + " Word");
194+
when(programRuleVariable.getSourceType())
195+
.thenReturn(ProgramRuleVariableSourceType.CALCULATED_VALUE);
196+
when(programRuleVariable.getValueType()).thenReturn(ValueType.TEXT);
172197

173198
List<ErrorReport> errorReports =
174199
programRuleVariableObjectBundleHook.validate(programRuleVariable, objectBundle);
@@ -180,8 +205,10 @@ void shouldFailValidationInvalidCountAndInvalidName() {
180205

181206
@Test
182207
void shouldFailValidationInvalidName() {
183-
when(programRuleVariable.getProgram()).thenReturn(program);
184208
when(objectBundle.getImportMode()).thenReturn(ImportStrategy.CREATE_AND_UPDATE);
209+
when(programRuleVariable.getSourceType())
210+
.thenReturn(ProgramRuleVariableSourceType.CALCULATED_VALUE);
211+
when(programRuleVariable.getValueType()).thenReturn(ValueType.TEXT);
185212
List<ErrorReport> errorReports;
186213

187214
for (String invalidKeyWord : PROGRAM_RULE_VARIABLE_NAME_INVALID_KEYWORDS) {
@@ -207,9 +234,11 @@ void shouldFailValidationInvalidName() {
207234

208235
@Test
209236
void shouldPassValidationWithValidName() {
210-
when(programRuleVariable.getProgram()).thenReturn(program);
211-
when(programRuleVariable.getName()).thenReturn("WordAndWord");
212237
when(objectBundle.getImportMode()).thenReturn(ImportStrategy.CREATE_AND_UPDATE);
238+
when(programRuleVariable.getName()).thenReturn("WordAndWord");
239+
when(programRuleVariable.getSourceType())
240+
.thenReturn(ProgramRuleVariableSourceType.CALCULATED_VALUE);
241+
when(programRuleVariable.getValueType()).thenReturn(ValueType.TEXT);
213242

214243
List<ErrorReport> errorReports =
215244
programRuleVariableObjectBundleHook.validate(programRuleVariable, objectBundle);
@@ -221,4 +250,136 @@ void shouldPassValidationWithValidName() {
221250
programRuleVariableObjectBundleHook.validate(programRuleVariable, objectBundle);
222251
assertEquals(0, errorReports1.size());
223252
}
253+
254+
static Stream<ProgramRuleVariableSourceType> dataElementSourceTypes() {
255+
return ProgramRuleVariableSourceType.getDataTypes().stream();
256+
}
257+
258+
@ParameterizedTest
259+
@MethodSource("dataElementSourceTypes")
260+
void shouldFailForAllDataElementSourceTypesWhenDataElementIsNull(
261+
ProgramRuleVariableSourceType sourceType) {
262+
when(programRuleVariable.getSourceType()).thenReturn(sourceType);
263+
when(programRuleVariable.getDataElement()).thenReturn(null);
264+
265+
List<ErrorReport> errorReports =
266+
programRuleVariableObjectBundleHook.validate(programRuleVariable, objectBundle);
267+
268+
assertTrue(errorReports.stream().anyMatch(e -> e.getErrorCode().equals(E4059)));
269+
}
270+
271+
@Test
272+
void shouldPassWhenDataElementSourceTypeHasDataElement() {
273+
when(programRuleVariable.getSourceType())
274+
.thenReturn(ProgramRuleVariableSourceType.DATAELEMENT_CURRENT_EVENT);
275+
when(programRuleVariable.getDataElement()).thenReturn(new DataElement());
276+
277+
List<ErrorReport> errorReports =
278+
programRuleVariableObjectBundleHook.validate(programRuleVariable, objectBundle);
279+
280+
assertEquals(0, errorReports.size());
281+
}
282+
283+
@Test
284+
void shouldFailWhenTeAttributeSourceTypeHasNoAttribute() {
285+
when(programRuleVariable.getSourceType())
286+
.thenReturn(ProgramRuleVariableSourceType.TEI_ATTRIBUTE);
287+
when(programRuleVariable.getAttribute()).thenReturn(null);
288+
289+
List<ErrorReport> errorReports =
290+
programRuleVariableObjectBundleHook.validate(programRuleVariable, objectBundle);
291+
292+
assertEquals(1, errorReports.size());
293+
assertTrue(errorReports.stream().anyMatch(e -> e.getErrorCode().equals(E4089)));
294+
}
295+
296+
@Test
297+
void shouldPassWhenTeAttributeSourceTypeHasAttribute() {
298+
when(programRuleVariable.getSourceType())
299+
.thenReturn(ProgramRuleVariableSourceType.TEI_ATTRIBUTE);
300+
when(programRuleVariable.getAttribute()).thenReturn(new TrackedEntityAttribute());
301+
302+
List<ErrorReport> errorReports =
303+
programRuleVariableObjectBundleHook.validate(programRuleVariable, objectBundle);
304+
305+
assertEquals(0, errorReports.size());
306+
}
307+
308+
@Test
309+
void shouldPassWhenCalculatedValueSourceTypeHasValueType() {
310+
when(programRuleVariable.getSourceType())
311+
.thenReturn(ProgramRuleVariableSourceType.CALCULATED_VALUE);
312+
when(programRuleVariable.getValueType()).thenReturn(ValueType.TEXT);
313+
314+
List<ErrorReport> errorReports =
315+
programRuleVariableObjectBundleHook.validate(programRuleVariable, objectBundle);
316+
317+
assertEquals(0, errorReports.size());
318+
}
319+
320+
@Test
321+
void shouldFailWhenSourceTypeIsMissing() {
322+
when(programRuleVariable.getSourceType()).thenReturn(null);
323+
324+
List<ErrorReport> errorReports =
325+
programRuleVariableObjectBundleHook.validate(programRuleVariable, objectBundle);
326+
327+
assertEquals(1, errorReports.size());
328+
assertTrue(errorReports.stream().anyMatch(e -> e.getErrorCode().equals(E4090)));
329+
}
330+
331+
@Test
332+
void shouldFailWhenProgramStageMissingForProgramStageDataElementSourceType() {
333+
when(programRuleVariable.getSourceType())
334+
.thenReturn(ProgramRuleVariableSourceType.DATAELEMENT_NEWEST_EVENT_PROGRAM_STAGE);
335+
when(programRuleVariable.getDataElement()).thenReturn(new DataElement());
336+
when(programRuleVariable.getProgramStage()).thenReturn(null);
337+
338+
List<ErrorReport> errorReports =
339+
programRuleVariableObjectBundleHook.validate(programRuleVariable, objectBundle);
340+
341+
assertEquals(1, errorReports.size());
342+
assertTrue(errorReports.stream().anyMatch(e -> e.getErrorCode().equals(E4091)));
343+
}
344+
345+
@Test
346+
void shouldPassWhenDataElementAndProgramStagePresentForProgramStageSourceType() {
347+
when(programRuleVariable.getSourceType())
348+
.thenReturn(ProgramRuleVariableSourceType.DATAELEMENT_NEWEST_EVENT_PROGRAM_STAGE);
349+
when(programRuleVariable.getDataElement()).thenReturn(new DataElement());
350+
when(programRuleVariable.getProgramStage()).thenReturn(new ProgramStage());
351+
352+
List<ErrorReport> errorReports =
353+
programRuleVariableObjectBundleHook.validate(programRuleVariable, objectBundle);
354+
355+
assertEquals(0, errorReports.size());
356+
}
357+
358+
@Test
359+
void shouldFailWhenCalculatedValueHasNoValueType() {
360+
when(programRuleVariable.getSourceType())
361+
.thenReturn(ProgramRuleVariableSourceType.CALCULATED_VALUE);
362+
when(programRuleVariable.getValueType()).thenReturn(null);
363+
364+
List<ErrorReport> errorReports =
365+
programRuleVariableObjectBundleHook.validate(programRuleVariable, objectBundle);
366+
367+
assertEquals(1, errorReports.size());
368+
assertTrue(errorReports.stream().anyMatch(e -> e.getErrorCode().equals(E4092)));
369+
}
370+
371+
@Test
372+
void shouldFailWhenBothDataElementAndProgramStageMissingForProgramStageSourceType() {
373+
when(programRuleVariable.getSourceType())
374+
.thenReturn(ProgramRuleVariableSourceType.DATAELEMENT_NEWEST_EVENT_PROGRAM_STAGE);
375+
when(programRuleVariable.getDataElement()).thenReturn(null);
376+
when(programRuleVariable.getProgramStage()).thenReturn(null);
377+
378+
List<ErrorReport> errorReports =
379+
programRuleVariableObjectBundleHook.validate(programRuleVariable, objectBundle);
380+
381+
assertEquals(2, errorReports.size());
382+
assertTrue(errorReports.stream().anyMatch(e -> e.getErrorCode().equals(E4059)));
383+
assertTrue(errorReports.stream().anyMatch(e -> e.getErrorCode().equals(E4091)));
384+
}
224385
}

‎dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/SchemaBasedControllerTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ class SchemaBasedControllerTest extends DhisControllerConvenienceTest {
7575
"jobConfiguration", // API requires configurable=true
7676
"messageConversation", // needs recipients (not a required field)
7777
"programRuleAction", // needs DataElement and TrackedEntityAttribute
78+
"programRuleVariable", // needs DataElement and TrackedEntityAttribute
7879
"validationRule", // generator insufficient (embedded fields)
7980
"programStage", // body request does not include mandatory field programId
8081
"programStageWorkingList", // same reason as programStage
@@ -116,7 +117,7 @@ void testCreateAndDeleteSchemaObjects() {
116117
assertStatus(HttpStatus.OK, DELETE(endpoint + "/" + uid));
117118
}
118119
}
119-
assertTrue(testedSchemas >= 57, "make sure we actually test schemas");
120+
assertTrue(testedSchemas >= 56, "make sure we actually test schemas");
120121
}
121122

122123
/** Uses the created instance to test the {@code /gist} endpoint list. */

0 commit comments

Comments
 (0)