mayope / keycloakmigration

Manage your Keycloak configuration with code.
https://mayope.net
MIT License
108 stars 22 forks source link

Add support for the 'addExecutionFlow' method #36

Closed ghilainm closed 1 year ago

ghilainm commented 2 years ago

Currently, you can add flow and execution to a flow. But you cannot add an execution flow to a flow if I am not wrong :).

Screenshot from 2021-11-09 13-20-08

The idea would be able to be able configure Browser like flow which have nested execution flows

Screenshot from 2021-11-09 16-28-29 .

klg71 commented 2 years ago

Hey ghilainm, Thanks for taking the time and opening this issue :) This is definitely a good change to implement but I fear it might take a couple of weeks before I can look into it. If you already have a solution in mind you are welcome to open a Pull Request and I will look into it as soon as possible :)

ghilainm commented 2 years ago

I tried opening the source code but quickly run into two issues:

Any idea of the issue?

klg71 commented 2 years ago

Which dependencies have invalid signatures and which unit tests are stuck?

The stuck-unittests could be explainable if the local Keycloak is not started.

ghilainm commented 2 years ago

So the signature issue vanished, don't know why sorry :(.

Two other errors:

For the doc:

Task :docsbuild:buildDocs FAILED bin/hugo: 1: Syntax error: word unexpected (expecting ")")

For the tests: Some kind of recursive exception seems to happen don't know the root cause (tested with two Gradle versions) (see test-2.txt):

Exception in thread "Test worker" java/lang/StackOverflowError

ghilainm commented 2 years ago

@klg71 I have created a fork of the project and did the following:

It seems to work a bit better on my side. No more Stackoverflow, which is already good, but 26 tests fail because you try to mock/inspect a final class.

-> https://app.circleci.com/pipelines/github/ghilainm/keycloakmigration?branch=master

Would you consider integrating those changes (after test fix?)

klg71 commented 2 years ago

My best guess for the failing tests are because you upgraded "io.mockk:mockk:1.12.0" but not "com.nhaarman.mockitokotlin2:mockito-kotlin:2.1.0". AFAIK the both depend on bytebuddy and mockitokotlin doesnt't work with the new version. So my suggestion would be to update mockito-kotlin to or if this isn't possible rollblack the mockk version :)

ghilainm commented 2 years ago

@klg71 I have replaced com.nhaarman.mockitokotlin2 by org.mockito.kotlin:mockito-kotlin which seems to be more standard :).

I am trying to fix this final class mocking stuff atm.

klg71 commented 2 years ago

Hey ghilainm i am currently fixing the master build, cause I found a couple of issues there too. If you could wait 1-2 hours for me to fix it you could start with a clean building master. Sry for the inconvieniences :/

ghilainm commented 2 years ago

I have already done some improvement which fixed the unit tests issues on my side -> https://app.circleci.com/pipelines/github/ghilainm/keycloakmigration

klg71 commented 2 years ago

Thanks for your work but these are so many changes that it would be impossible for me to review. I fixed the build issues on the master and you can start to work on the "addExecutionFlow". Can you please verify that the updated master builds on your machine?

ghilainm commented 2 years ago

@klg71 In fact most of the changes are packages changes so that would be rather quick, I think it is still worth taking a look at it.

klg71 commented 2 years ago

I think its definitely worth looking at and I am really thankful for your commitment at this project. I would suggest we schedule a call to clear up the situtation. Can you send me an E-Mail to klg71@web.de so I can send you over my number? :)

ghilainm commented 2 years ago

I have closed the MR as you did the cleanup yourself it seems :D.

klg71 commented 2 years ago

Yeah I did and I hope you accept my apology for my first reaction, that the build was not running. Communication should have been definitely better on my side :/