Eureka Team Code Uniformity

Purpose

This page contains uniform rules for Eureka development

Helpful files

• Checkstyle

• Intellij Idea

General

Sonarcloud rules are important

• If the current version of SonarCloud does not support any functionality, use a suitable implementation
  For example, instead of using .toList() - use .collect(toList()) or .collect(toUnmodifiableList()) for an immutable list.

• If sonar provides incorrect validation, it must be discussed with a team and suppressed with a comment

Methods

Length

Method length must be less than 25 lines in most cases, excluding the declaration line and closing brace. If implementation takes more lines - try to decomposite method/class.

Naming

1. Each method must start with a verb in imperative form: startProcessing, doOperation, performAction, mapResources

2. The return value is boolean: is, exists, has. However, a noun can go first if it simplifies the method name: usernameExists or existsByUsername

3. If the method is aimed to validate and then throw a validation exception, use verbs: verify, check, assert

4. onCreate, onUpdate are valid names for listener methods

Methods/operation usage rules

• Use isNull() / isNotNull() methods in as a method reference in lambdas, otherwise != null or == null is more clear

Arrays.asList(1, 2, null).stream()
  .filter(Objects::nonNull)
  .collect(toSet());

if (Objects.nonNull(object)) {
  doAction(object);
}

if (object != null) {
  doAction(object);
}

• Use .forEach method if it shortens the code to a single line, otherwise, the classic for-each cycle is the more convenient and faster

list.forEach(element -> {
  doAction1(element);
  doAction2(element);
  doAction3(element);
});

list.forEach(element -> doSomethingWithElement(element));

void doSomethingWithElement(Object element) {
  doAction1(element);
  doAction2(element);
  doAction3(element);
}

for (var element : list) {
  doAction1(element);
  doAction2(element);
  doAction3(element);
}

• Use if-return instead of if-else if possible

public void doConditionalOperation(Object value) {
  Object result;
  if (value != null) {
    result = mapToEntity(value);
  } else {
    throw new IllegalStateException("Cannot be null");
  }
}

public void doConditionalOperation(Object value) {
  if (value == null) {
    throw new IllegalStateException("Cannot be null");
  }
  return mapToEntity(value);
}

public void perform(Object value) {

  if (exists(value)) {
    var updatedValue = updateValue(value);
    log.info("Object is updated: value = {}", value);
  } else {
    var createdValue = createValue(value);
    log.info("Object is created: value = {}", value);
  }
}

public void perform(Object value) {
  if (exists(value)) {
    var updatedValue = updateValue(value);
    log.info("Object is updated [value: {}]", value);
    return;
  }

  var createdValue = createValue(value);
  log.info("Object is created [value: {}]", value);
}

• Use Optional for public methods to nullable objects, private methods can return a null value, if it will be more convenient to use in code

Optional<Object> findById(UUID id);

Object findById(UUID id);

Optional<Object> findById(UUID id);

public Object getById(UUID id) {
  if (!repository.existsById(id)) {
    throw new EntityNotFoundException("Entity is not found by id: " + id);
  }
  // continue implementation
  return foundObject;
}

/**
* Loads reference data. This method should be idempotent.
*
* @param id - entity identifier
* @return found object by id
* @throws EntityNotFoundException - if entity is not found by id
*/
public Object getById(UUID id) {
  return repository.getReferencyById(id);
  if (!repository.existsById(id)) {
    throw new EntityNotFoundException("Entity is not found by id: " + id);
  }
  // continue implementation
  return foundObject;
}

private Map<UUID, Object> cache;

public Object getValueSafe(UUID id) {
  return cache.findById(id).orElse(getEntityById(id));
}

private Object getEntityById(UUID id) {
  // retrieve entity using http call (can take 1-10 seconds)
}

private Map<UUID, Object> cache;

public Object getValueSafe(UUID id) {
  return cache.findById(id).orElseGet(() -> getEntityById(id));
}

private Object getEntityById(UUID id) {
  // retrieve entity using http call (can take 1-10 seconds)
}

Optional.ofNullable(value).ifPresent(value -> doSomething(value))

if (value != null) {
  doSomething(value);
}

• The ternary operator is good to use for the return value (can be split into 3 lines with the operator at the new line) or to declare a variable (if it fits a single line)

public <T> T findFirst(List<T> list) {
  return CollectionUtils.isNotEmpty(list) ? Optional.of(list.get(0)) : Optional.empty();
}

public <T> T findFirst(List<T> list) {
  return CollectionUtils.isNotEmpty(list) 
    ? Optional.of(list.get(0)) 
    : Optional.empty();
}

public <T> String findFirstAsString(List<T> list) {
  var value = CollectionUtils.isNotEmpty(list) ? Optional.of(list.get(0)) : Optional.empty();
  return value.map(element -> element.toString());
}

• Process negative conditions first using if-statement, positive flow must go to the end of a method

public void doConditionalOperation(Object value) {
  Object result;
  if (value != null) {
    result = mapToEntity(value);
  } else {
    throw new IllegalStateException("Cannot be null");
  }
}

public void doConditionalOperation(Object value) {
  if (value == null) {
    throw new IllegalStateException("Cannot be null");
  }
  return mapToEntity(value);
}

• Return an empty collection always if the method returns a collection

public <T> List<T> filterElements(List<T> list, Predicate<T> predicate) {
  if (list == null) {
    return Collections.emptyList();
  }
  
  return list.stream()
    .filter(predicate)
    .collect(Collectors.toList());
}

public <T> List<T> filterElements(List<T> list, Predicate<T> predicate) {
  if (list == null) {
    return null;
  }
  
  return list.stream()
    .filter(predicate)
    .collect(Collectors.toList());
}

public <T> List<T> filterElements(List<T> list, Predicate<T> predicate) {
  if (list == null) {
    return List.of();
  }
  
  return list.stream()
    .filter(predicate)
    .collect(Collectors.toList());
}

public <T> List<T> filterElements(List<T> list, Predicate<T> predicate) {
  if (list == null) {
    return Collections.emptyList();
  }
  
  return list.stream()
    .filter(predicate)
    .collect(Collectors.toList());
}

• Use static methods to indicate that class fields are not used, maybe later they can be considered to move to the utility class

@Log4j2
public class MyService {
  public Object performOperation(Object value) {
    return processValueWithoutContext(value);
  }
  
  private void processValueWithoutContext(value) {
    log.info("Processing record without context [value: {}]", value);
  }
}

@Log4j2
public class MyService {
  public Object performOperation(Object value) {
    return processValueWithoutContext(value);
  }
  
  private static void processValueWithoutContext(value) {
    log.info("Processing record without context [value: {}]", value);
  }
}

• Use static imports whenever it is possible

public <T> List<T> filterElements(List<T> list, Predicate<T> predicate) {
  if (list == null) {
    return Collections.emptyList();
  }
  
  return list.stream()
    .filter(predicate)
    .collect(Collectors.toList());
}

import static java.util.Collections.emptyList;
import static java.util.stream.Collectors.toList;

...

public <T> List<T> filterElements(List<T> list, Predicate<T> predicate) {
  if (list == null) {
    return emptyList();
  }
  
  return list.stream()
    .filter(predicate)
    .collect(toList());
}

import static java.util.List.of;
import static java.util.stream.Collectors.toList;

...

public List<Integer> filterElements(List<Integer> list, Predicate<Integer> predicate) {
  if (list == null) {
    // return some default value
    return of(1, 2, 3);
  }
  
  return list.stream()
    .filter(predicate)
    .collect(toList());
}

import static java.util.stream.Collectors.toList;

...

public List<Integer> filterElements(List<Integer> list, Predicate<Integer> predicate) {
  if (list == null) {
    // return some default value
    return of(1, 2, 3);
  }
  
  return list.stream()
    .filter(predicate)
    .collect(toList());
}

• Don’t modify source data to perform an action, it can be used further and those changes can lose initial information from the event/read operation.

• Prefer a more functional approach when the method always returns a new value

  • BeanUtils.copyProperties(), collections are copied by reference, deepCopy mode if it is required to copy collection fields too

  • The method can return modified value - if it is used for model construction (so-called with methods, or chain setters)

• Check utility method from folio-backend-tools

Logging

Log level = TRACE

• No need to provide logs for this level

Log level = DEBUG

• Debug payload request from HTTP method or Kafka, use suppliers for big objects: () -> getPayload()

• Make them be enabled using Actuator (implemented in Folio) or by providing additional log4j.properties for test resources with a disabled threshold filter.

• Indicate if important that entity/resource/record has been created/updated/deleted.

Log level = INFO

• Helpful operation indicators: job started/finished

• The payload must not be provided as short as possible: an identifier or name should be enough to identify the record

Log Level = WARN

• Errors and warnings that are not blocking the application from running, but can indicate business logic issues

Log Level = ERROR

• Errors that are blocking the application from running

Log format and examples

"{message}: {parameter1} = {value1}, {parameter2} = {value2}"

// examples
log.info("Operation execution finished: id = {}", UUID.randomId())

// there is no need to manyally specify tenant name, because it will be populated by Folio logger extension from values from FolioExecutionContext
log.info("Tenant is initialized")

// if serialization of value is not a straight-forward, use a consumer notation
log.debug("Value is created: value = {}", () -> asJsonString(someValue))

Unit tests

Unit tests must satisfy F.I.R.S.T principles.

Naming

{methodName}_{returnType:positive|negative}_{condition:inputValueIsNull}

Examples using @Nested class

@UnitTest
class Test {

  @Nested
  @DisplayName("findById")
  class FindById {
    
    @Test
    void positive() {};
    
    @Test
    void negative_entityIsNotFoundInRepository() {};
    
    @Test
    void negative_exceptionFromRepository() {};
    
    @Test
    void negative_failedToConstructId() {};
  }  
}

Examples without using @Nested class

@UnitTest
class Test {

  @Test
  void findEntityById_positive() {};
  
  @Test
  void findEntityById_negative_entityIsNotFoundInRepository() {};
  
  @Test
  void findEntityById_negative_exceptionFromRepository() {};
    
  @Test
  void findEntityById_negative_failedToConstructId() {};
}

• Tests (creating stubs): createApplicationDescriptor() → applicationDescriptor()

• Unit tests can be structured with a @Nestedclass for each method, however it is not mandatory

• Unit tests must use AssertJ library for uniformity, other assertion tools are less readable, but can be faster, however, there are no problems for big services using assertj

  • A reflection comparison can be used to ignore specific fields for model or the models without implemented equals method

  • containsExactly, containsExactrly in any order to check collection elements of retrieving them one by one

  • http://joel-costigliola.github.io/assertj/ - check other comparisons

Exception validation

Make sure that all significant exception parameters are verified.

• Exception type

• Exception message

• Exception response code

• Exception parameters (if present)

❗ Make sure that all parameters in the exception supplier method are defined not inside this lambda, otherwise it will produce sonar cloud code smell java:S5778 Only one method invocation is expected when testing runtime exceptions

Example for exception without parameters

@Test
void getEntity_positive() {
  var entityId = UUID.randomId();
  assertThatThrownBy(() -> service.getEntity(entityId))
    .isInstanceOf(EntityNotFoundException.class)
    .hasMessage("Unable to find entity with id %s", entityId);
}

Example for exception with parameters

@Test
void validateEntity_positive() {
  var expectedParameters = List.of(
    new Parameter().key("request.key").value("must not be null");
  );
  
  assertThatThrownBy(() -> service.validateEntity(request))
    .isInstanceOf(RequestValidationException.class)
    .hasMessage("Found validation errors in request")
    .satisfies(err -> assertThat(((RequestValidationException) err).getErrorParameters()).isEqualTo(expectedParams));
}

Parameterized testing

Parameterized test can be used for method with processing depending on incoming data, but always producing same response. Usually it is utility methods.

  @Nested
  @DisplayName("existsById")
      class ExistsById {
  
    private static final UUID ENTITY_ID = UUID.randomId();

    @ParameterizedTest
    @CsvSource({"true", "false"})
    void positive(boolean entityExistsResult) {
      when(repository.existsById(ENTITY_ID)).thenReturn(entityExistsResult);
      var result = service.existsById(ENTITY_ID);
      assertThat(result).isEqualTo(entityExistsResult);
    }
  }

Unit tests validation

• Mutation tests can be used to verify the quality of written unit tests

  • https://en.wikipedia.org/wiki/Mutation_testing

  • https://pitest.org/

• Parameterized tests must be used for utility classes or methods whenever possible (add example, simple guide)

  1. Property / Data-driven testing - as an additional type of validation

• Tests must be as simple as possible, and they must be fast (there is no need to test cache expiration for 12 seconds if it can be done using smaller values (100 ms as an example)

Integration tests

• Use API calls to verify behavior

• Always verify positive flow using integration tests

• All integrations with outside applications/databases performed using https://testcontainers.com/

• Real-service injections to the method are prohibited

  • In exceptional cases, they can be injected using @Authowired ProductionComponent component notation at class level

• Asserts must be as simple as possible

  mockMvc.perform(get("/entities")
      .header(TENANT, TENANT_ID)
    .andExpect(status().isOk())
    .andExpect(content().json(asJsonString(readTemplate("/json/responses/entity.json")), true)));

  mockMvc.perform(post("/entities")
      .content(capabilityToCreateAsJson)
      .header(TENANT, TENANT_ID)
      .contentType(APPLICATION_JSON))
    .andExpect(status().isCreated())
    .andExpect(content().json(asJsonString(readTemplate("/json/responses/entity.json"))))
    .andExpect(jsonPath("$.id").value(notNullValue()))
    .andExpect(jsonPath("$.metadata.createdBy").value(equalTo(USER_ID)))
    .andExpect(jsonPath("$.metadata.createdDate").value(notNullValue()))
    .andExpect(jsonPath("$.metadata.modifiedBy").doesNotExist())
    .andExpect(jsonPath("$.metadata.modifiedDate").doesNotExist());

• Verify that external data is created in outside service

  • Java HTTP client can be used to verify data in Keycloak

  • JDBC Helpers classes to verify database data performing native SQL requests, if it cannot be checked using API

  • Kong created routes can be verified using wiremock and keycloak native functionality mgr-tenant-entitlements: EntitilementRoutesIT