Feedback from code reviews

This commit is contained in:
2022-08-11 09:42:30 -05:00
parent 9a7acce3c6
commit 3b1f0b47c1
34 changed files with 486 additions and 196 deletions

View File

@ -30,6 +30,7 @@ import java.util.stream.Collectors;
import com.kingsrook.qqq.backend.core.exceptions.QException;
import com.kingsrook.qqq.backend.core.model.actions.metadata.MetaDataInput;
import com.kingsrook.qqq.backend.core.model.actions.metadata.MetaDataOutput;
import com.kingsrook.qqq.backend.core.model.metadata.frontend.AppTreeNode;
import com.kingsrook.qqq.backend.core.model.metadata.frontend.QFrontendAppMetaData;
import com.kingsrook.qqq.backend.core.utils.TestUtils;
import org.junit.jupiter.api.Test;
@ -84,7 +85,7 @@ class MetaDataActionTest
QFrontendAppMetaData peopleApp = apps.get(TestUtils.APP_NAME_PEOPLE);
assertThat(peopleApp.getChildren()).isNotEmpty();
Optional<QFrontendAppMetaData> greetingsAppUnderPeopleFromMapOptional = peopleApp.getChildren().stream()
Optional<AppTreeNode> greetingsAppUnderPeopleFromMapOptional = peopleApp.getChildren().stream()
.filter(e -> e.getName().equals(TestUtils.APP_NAME_GREETINGS)).findFirst();
assertThat(greetingsAppUnderPeopleFromMapOptional).isPresent();
@ -97,18 +98,18 @@ class MetaDataActionTest
///////////////////////////////////////////////
// assert against the hierarchical apps tree //
///////////////////////////////////////////////
List<QFrontendAppMetaData> appTree = result.getAppTree();
Set<String> appNamesInTopOfTree = appTree.stream().map(QFrontendAppMetaData::getName).collect(Collectors.toSet());
List<AppTreeNode> appTree = result.getAppTree();
Set<String> appNamesInTopOfTree = appTree.stream().map(AppTreeNode::getName).collect(Collectors.toSet());
assertThat(appNamesInTopOfTree).contains(TestUtils.APP_NAME_PEOPLE);
assertThat(appNamesInTopOfTree).contains(TestUtils.APP_NAME_MISCELLANEOUS);
assertThat(appNamesInTopOfTree).doesNotContain(TestUtils.APP_NAME_GREETINGS);
Optional<QFrontendAppMetaData> peopleAppOptional = appTree.stream()
Optional<AppTreeNode> peopleAppOptional = appTree.stream()
.filter(e -> e.getName().equals(TestUtils.APP_NAME_PEOPLE)).findFirst();
assertThat(peopleAppOptional).isPresent();
assertThat(peopleAppOptional.get().getChildren()).isNotEmpty();
Optional<QFrontendAppMetaData> greetingsAppUnderPeopleFromTree = peopleAppOptional.get().getChildren().stream()
Optional<AppTreeNode> greetingsAppUnderPeopleFromTree = peopleAppOptional.get().getChildren().stream()
.filter(e -> e.getName().equals(TestUtils.APP_NAME_GREETINGS)).findFirst();
assertThat(greetingsAppUnderPeopleFromTree).isPresent();

View File

@ -26,6 +26,7 @@ import java.math.BigDecimal;
import com.kingsrook.qqq.backend.core.exceptions.QException;
import com.kingsrook.qqq.backend.core.model.data.testentities.Item;
import com.kingsrook.qqq.backend.core.model.data.testentities.ItemWithPrimitives;
import com.kingsrook.qqq.backend.core.model.metadata.fields.DisplayFormat;
import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldMetaData;
import com.kingsrook.qqq.backend.core.model.metadata.fields.QFieldType;
import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData;
@ -177,6 +178,8 @@ class QRecordEntityTest
///////////////////////////////////////////////////////////////
// assert about attributes that came from @QField annotation //
///////////////////////////////////////////////////////////////
assertEquals("SKU", qTableMetaData.getField("sku").getLabel());
assertEquals(DisplayFormat.COMMAS, qTableMetaData.getField("quantity").getDisplayFormat());
assertTrue(qTableMetaData.getField("sku").getIsRequired());
assertFalse(qTableMetaData.getField("quantity").getIsEditable());
assertEquals("is_featured", qTableMetaData.getField("featured").getBackendName());

View File

@ -25,6 +25,7 @@ package com.kingsrook.qqq.backend.core.model.data.testentities;
import java.math.BigDecimal;
import com.kingsrook.qqq.backend.core.model.data.QField;
import com.kingsrook.qqq.backend.core.model.data.QRecordEntity;
import com.kingsrook.qqq.backend.core.model.metadata.fields.DisplayFormat;
/*******************************************************************************
@ -32,13 +33,13 @@ import com.kingsrook.qqq.backend.core.model.data.QRecordEntity;
*******************************************************************************/
public class Item extends QRecordEntity
{
@QField(isRequired = true)
@QField(isRequired = true, label = "SKU")
private String sku;
@QField()
private String description;
@QField(isEditable = false)
@QField(isEditable = false, displayFormat = DisplayFormat.COMMAS)
private Integer quantity;
private BigDecimal price;

View File

@ -60,21 +60,8 @@ public class Auth0AuthenticationModuleTest
/*******************************************************************************
** Test an expired token where 'now' is set to a time that would not require it to be
** re-checked, so it'll show as valid
**
*******************************************************************************/
@Test
public void testLastTimeCheckedNow()
{
assertTrue(testLastTimeChecked(Instant.now(), UNDECODABLE_TOKEN), "A session just checked 'now' should always be valid");
}
/*******************************************************************************
** Test an expired token where 'now' is set to a time that would not require it to be
** re-checked, so it'll show as valid
** Test a token where last-checked is set to a time that would not require it to be
** re-checked, so it'll show as valid no matter what the token is.
**
*******************************************************************************/
@Test
@ -87,8 +74,8 @@ public class Auth0AuthenticationModuleTest
/*******************************************************************************
** Test an expired token where 'now' is set to a time that would require it to be
** re-checked
** Test a token where last-checked is set to a time that would require it to be
** re-checked, so it'll show as invalid.
**
*******************************************************************************/
@Test
@ -101,8 +88,8 @@ public class Auth0AuthenticationModuleTest
/*******************************************************************************
** Test an expired token where 'now' is set to a time that would require it to be
** re-checked
** Test a token where last-checked is past the threshold, so it'll get re-checked,
** and will fail.
**
*******************************************************************************/
@Test

View File

@ -55,7 +55,10 @@ class StreamedETLProcessTest
RunProcessOutput result = new RunProcessAction().execute(request);
assertNotNull(result);
assertTrue(result.getRecords().stream().allMatch(r -> r.getValues().containsKey("id")), "records should have an id, set by the process");
///////////////////////////////////////////////////////////////////////
// since this is streamed, assert there are no records in the output //
///////////////////////////////////////////////////////////////////////
assertTrue(result.getRecords().isEmpty());
assertTrue(result.getException().isEmpty());
}
@ -77,12 +80,14 @@ class StreamedETLProcessTest
// define our mapping from destination-table field names to source-table field names //
///////////////////////////////////////////////////////////////////////////////////////
QKeyBasedFieldMapping mapping = new QKeyBasedFieldMapping().withMapping("name", "firstName");
// request.addValue(StreamedETLProcess.FIELD_MAPPING_JSON, JsonUtils.toJson(mapping.getMapping()));
request.addValue(StreamedETLProcess.FIELD_MAPPING_JSON, JsonUtils.toJson(mapping));
RunProcessOutput result = new RunProcessAction().execute(request);
assertNotNull(result);
assertTrue(result.getRecords().stream().allMatch(r -> r.getValues().containsKey("id")), "records should have an id, set by the process");
///////////////////////////////////////////////////////////////////////
// since this is streamed, assert there are no records in the output //
///////////////////////////////////////////////////////////////////////
assertTrue(result.getRecords().isEmpty());
assertTrue(result.getException().isEmpty());
}

View File

@ -53,7 +53,6 @@ class ValueUtilsTest
@Test
void testGetValueAsString() throws QValueException
{
//noinspection ConstantConditions
assertNull(ValueUtils.getValueAsString(null));
assertEquals("", ValueUtils.getValueAsString(""));
assertEquals(" ", ValueUtils.getValueAsString(" "));
@ -164,26 +163,28 @@ class ValueUtilsTest
@Test
void testGetValueAsLocalDate() throws QValueException
{
LocalDate expected = LocalDate.of(1980, Month.MAY, 31);
assertNull(ValueUtils.getValueAsLocalDate(null));
assertNull(ValueUtils.getValueAsLocalDate(""));
assertNull(ValueUtils.getValueAsLocalDate(" "));
assertEquals(LocalDate.of(1980, Month.MAY, 31), ValueUtils.getValueAsLocalDate(LocalDate.of(1980, 5, 31)));
assertEquals(LocalDate.of(1980, Month.MAY, 31), ValueUtils.getValueAsLocalDate(new java.sql.Date(80, 4, 31)));
assertEquals(expected, ValueUtils.getValueAsLocalDate(LocalDate.of(1980, 5, 31)));
assertEquals(expected, ValueUtils.getValueAsLocalDate(new java.sql.Date(80, 4, 31)));
//noinspection MagicConstant
assertEquals(LocalDate.of(1980, Month.MAY, 31), ValueUtils.getValueAsLocalDate(new java.util.Date(80, 4, 31)));
assertEquals(LocalDate.of(1980, Month.MAY, 31), ValueUtils.getValueAsLocalDate(new java.util.Date(80, Calendar.MAY, 31)));
assertEquals(LocalDate.of(1980, Month.MAY, 31), ValueUtils.getValueAsLocalDate(new java.util.Date(80, Calendar.MAY, 31, 12, 0)));
assertEquals(LocalDate.of(1980, Month.MAY, 31), ValueUtils.getValueAsLocalDate(new java.util.Date(80, Calendar.MAY, 31, 4, 0)));
assertEquals(LocalDate.of(1980, Month.MAY, 31), ValueUtils.getValueAsLocalDate(new java.util.Date(80, Calendar.MAY, 31, 22, 0)));
assertEquals(expected, ValueUtils.getValueAsLocalDate(new java.util.Date(80, 4, 31)));
assertEquals(expected, ValueUtils.getValueAsLocalDate(new java.util.Date(80, Calendar.MAY, 31)));
assertEquals(expected, ValueUtils.getValueAsLocalDate(new java.util.Date(80, Calendar.MAY, 31, 12, 0)));
assertEquals(expected, ValueUtils.getValueAsLocalDate(new java.util.Date(80, Calendar.MAY, 31, 4, 0)));
assertEquals(expected, ValueUtils.getValueAsLocalDate(new java.util.Date(80, Calendar.MAY, 31, 22, 0)));
//noinspection MagicConstant
assertEquals(LocalDate.of(1980, Month.MAY, 31), ValueUtils.getValueAsLocalDate(new GregorianCalendar(1980, 4, 31)));
assertEquals(LocalDate.of(1980, Month.MAY, 31), ValueUtils.getValueAsLocalDate(new GregorianCalendar(1980, Calendar.MAY, 31)));
assertEquals(LocalDate.of(1980, Month.MAY, 31), ValueUtils.getValueAsLocalDate(LocalDateTime.of(1980, 5, 31, 12, 0)));
assertEquals(LocalDate.of(1980, Month.MAY, 31), ValueUtils.getValueAsLocalDate(LocalDateTime.of(1980, 5, 31, 4, 0)));
assertEquals(LocalDate.of(1980, Month.MAY, 31), ValueUtils.getValueAsLocalDate(LocalDateTime.of(1980, 5, 31, 22, 0)));
assertEquals(LocalDate.of(1980, Month.MAY, 31), ValueUtils.getValueAsLocalDate(LocalDateTime.of(1980, Month.MAY, 31, 12, 0)));
assertEquals(LocalDate.of(1980, Month.MAY, 31), ValueUtils.getValueAsLocalDate("1980-05-31"));
assertEquals(LocalDate.of(1980, Month.MAY, 31), ValueUtils.getValueAsLocalDate("05/31/1980"));
assertEquals(expected, ValueUtils.getValueAsLocalDate(new GregorianCalendar(1980, 4, 31)));
assertEquals(expected, ValueUtils.getValueAsLocalDate(new GregorianCalendar(1980, Calendar.MAY, 31)));
assertEquals(expected, ValueUtils.getValueAsLocalDate(LocalDateTime.of(1980, 5, 31, 12, 0)));
assertEquals(expected, ValueUtils.getValueAsLocalDate(LocalDateTime.of(1980, 5, 31, 4, 0)));
assertEquals(expected, ValueUtils.getValueAsLocalDate(LocalDateTime.of(1980, 5, 31, 22, 0)));
assertEquals(expected, ValueUtils.getValueAsLocalDate(LocalDateTime.of(1980, Month.MAY, 31, 12, 0)));
assertEquals(expected, ValueUtils.getValueAsLocalDate("1980-05-31"));
assertEquals(expected, ValueUtils.getValueAsLocalDate("05/31/1980"));
assertThrows(QValueException.class, () -> ValueUtils.getValueAsLocalDate("a"));
assertThrows(QValueException.class, () -> ValueUtils.getValueAsLocalDate("a,b"));