TNG / ArchUnit

A Java architecture test library, to specify and assert architecture rules in plain Java
http://archunit.org
Apache License 2.0
3.23k stars 297 forks source link

[suggestion] Allow the mechanism for creating a file name for frozen violations to be specified via an SPI or similar. #1045

Open danhaywood opened 1 year ago

danhaywood commented 1 year ago

this follows on (somewhat) https://github.com/TNG/ArchUnit/issues/1025.

We have a modular monolith, and have set up archunit to run for each module. We are using frozen violations to manage the technical debt in each module, to prevent new violations from being added and to chip away at old ones.

In some circumstances Archunit will recreate the guid files with the current set of violations, which in theory is a good thing but we find that the guid file can sometimes change. Perhaps our workflow is wrong, but at any rate, when multiple developers on current feature branches, we find we get a bunch of git conflicts that need to be resolved.

What we think would work for us is if the file name storing the violations for a rule were deterministic rather than a guid. Looking at the code, this is done at https://github.com/TNG/ArchUnit/blob/main/archunit/src/main/java/com/tngtech/archunit/library/freeze/ViolationStoreFactory.java#L166 , so the suggestion is to provide an SPI that makes this strategy pluggable.

danhaywood commented 1 year ago

On closer inspection, I see that the code in question is in TextFileBasedViolationStore, so perhaps this is already pluggable. Will dig a bit deeper.

danhaywood commented 1 year ago

I've raised a PR https://github.com/TNG/ArchUnit/pull/1046 for your consideration.

danhaywood commented 1 year ago

In the meantime, I've copied out the TextFileBasedViolationStore from the associated PR TNG#1046, and I've created my subclass to create deterministic file names:

import java.util.ArrayList;
import java.util.List;

import com.tngtech.archunit.thirdparty.com.google.common.base.Joiner;
import com.tngtech.archunit.thirdparty.com.google.common.base.Splitter;

public class DeterministicTextFileBasedViolationStore extends TextFileBasedViolationStore {

    // 100 (directory) + 155 + 4 (extension) = 259  ... Windows has max limit of 260 chars.
    public static final int MAX_LENGTH = 155;

    protected String newRuleFileName(String description) {
        String s = description.replace(' ', '_').replaceAll("[^a-zA-Z0-9_-]", "");
        if(s.length() > MAX_LENGTH) {
            List<String> parts = new ArrayList<>(Splitter.on("_").splitToList(s));
            String candidate = Joiner.on("_").join(parts);
            int partToTruncate = parts.size() - 1;
            while( candidate.length() > MAX_LENGTH) {
                char firstChar = parts.get(partToTruncate).charAt(0);
                parts.set(partToTruncate, firstChar+"");
                partToTruncate--;
                candidate = Joiner.on("_").join(parts);
            }
            s = candidate;
        }
        return s  + ".txt";
    }
}

with

import lombok.RequiredArgsConstructor;

import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.EnumSource;

class DeterministicTextFileBasedViolationStore_Test {

    @ParameterizedTest
    @EnumSource(Scenario.class)
    void newRuleFileName(Scenario scenario) {
        Assertions.assertThat(new DeterministicTextFileBasedViolationStore().newRuleFileName(scenario.input)).isEqualTo(scenario.expected);
    }

    @Test
    void long_rule_name() {
        Assertions.assertThat(
                new DeterministicTextFileBasedViolationStore().newRuleFileName(
                        "methods_that_have_name_matching_find_and_have_name_not_matching_findOrCreate_and_are_declared_in_classes_that_are_annotated_with_DomainService_and_are_declared_in_classes_that_have_name_matching_Repository_should_have_raw_return_type_either_Optional_or_Collection"))
                    .isEqualTo(
                            // <=155 characters (plus 100 for the directory name, so less than 260 overall)
                        "methods_that_have_name_matching_find_and_have_name_not_matching_findOrCreate_and_are_declared_in_classes_that_a_a_w_D_a_a_d_i_c_t_h_n_m_R_s_h_r_r_t_e_O_o_C.txt");
    }

    @RequiredArgsConstructor
    enum Scenario {
        lower_accepted          ("abc",   "abc.txt"),
        upper_accepted          ("ABC",   "ABC.txt"),
        number_accepted         ("123",   "123.txt"),
        underscore_accepted     ("a_123", "a_123.txt"),
        hyphen_accepted         ("a-123", "a-123.txt"),
        space_becomes_underscore("a bc",  "a_bc.txt"),
        colon_stripped          ("a:123", "a123.txt"),
        percentage_stripped     ("a%123", "a123.txt"),
        comma_stripped          ("a,123", "a123.txt"),
        period_stripped         ("a.123", "a123.txt"),
        ;
        private final String input;
        private final String expected;
    }
}