Change Request: ViolationStore-API should give access to structured violation data

[AndreasIgelCC]

Hi at all,

first: archunit is Great and this issue is just my way to find a way to improve it for further usages!

When trying to create a custom violation store, the data provided makes some problems. As I understood, you have only the final violation messages for storing and not the violation object itself. By that it is difficult to store violations in a different format.

Idea is to change the ViolationStore API in a way to give access to the original data. In that way specific yaml,json and xml exports are possible. This could be automatically imported into issue systems.

Best regards, Andreas

PS: if you like and you are interested in the change, I could do I proposal in shape of a Pull Request.

[codecholeric]

Glad you like ArchUnit and happy for new ideas to make it even better :smiley: I'm principally open to making the ViolationStore API more powerful, but I'm not sure I completely understand it yet. What exactly do you mean by "violation object"? I don't think ArchUnit ever has one of those. All it has are those ConditionEvents, do you mean those? At the moment there are some workarounds on the outer side of store(List<String> violations) though (at least I remember ensuring Unix line breaks in each single violation, because that caused problems cross-platform). The idea was that implementors don't have to deal with that :thinking: So I don't know how that would map to the other API. And also if it could be extended in a backwards compatible way or if it would be breaking...

[AndreasIgelCC]

Thank you for your answer and your thoughts. The last days I had some time to look on this. It really seams not to be easy, because the whole store is based on strings and the evaluation results seems not to contain the wished detailed information. Furthermore archunit seems to support even Java1.7. So migration of interfaces are a bit more complicated. So from my point of view, the following changes have to be made to get that issue done:

1. New store interface with a switch from String to evaluationResult. I would put the old interface on deprecated and the storeFactory has to support both (Java 1.7 limitation if it should not be a breaking change)
2. Migration of default Textstore and Categorisation to evaluationResult. Maybe just using the report for that, have to be looked deeper.
3. Extending the evaluationResult to contain the locations. (Maybe it is already there, but found only the description text by now)
4. Adding a new store for supporting json format.

I would propose to make a PullRequest and you could have a deeper look on the needed changes. When I look on the points above, that could be a bigger one. If it fits your ideas we can discuss that and if you want to go that way! Is that ok for you?

Best Regards, Andreas

[codecholeric]

One important thing to keep in mind about the location is that for ArchUnit it's not always a single file. Take a package cycle violation for example, class a.A1 targets b.B1 and b.B2 targets a.A2. You can't really assign one single location to this, because all involved classes together form the violation and no class alone is the culprit. So, it would probably be good to look into different EvaluationResults first and ponder how this would be mapped in this case (e.g. what is "the location" of a violation in such a case).

Also, I'm not sure the old API needs to be deprecated, because it might also be simpler for some cases. E.g. if I would just want to store the violations in a relational DB for example I could use the same API. And it offers some benefits, e.g. that you don't need to worry about line breaks when you're developing on different platforms. Maybe both APIs could be offered :thinking:

Feel free to create a PR :slightly_smiling_face: But, I would try to go for the most risky things first. E.g. how would the processing of EvaluationResult look like to get the structured data. Does it really work for typical library rules like slices()...should().beFreeOfCycles() or how would it look like for custom ConditionEvents? For example that correspondingObject can in theory be chosen completely free. How should we handle generic objects? Things like that. If that can be solved I'm sure the rest will be fine as well :man_shrugging: But for these points I'm not sure yet...

[AndreasIgelCC]

Thank you for the response, the explanations and the ideas. The 1:N connection between rule and violation I have been aware of. From my point of view this is considered already by the List of possible Results. So the resulting violations of one rule are all part of one evaluationResult.

After reading your explanations I agree, that starting with the structured EvaluationResult is a good idea! And I guess it would be easier to discuss, when having the first code to do a review. I will tell you as soon as I am ready with that!