arquillian / arquillian-algeron

12 stars 8 forks source link

@PactBroker doesn't work anymore #70

Closed scheuchzer closed 7 years ago

scheuchzer commented 7 years ago

In Alpha6 an exception gets thrown when using the PactBroker annotation:

Caused by: java.lang.NoSuchMethodException: org.arquillian.algeron.pact.provider.core.loader.pactbroker.PactBrokerLoader.<init>(org.arquillian.algeron.pact.provider.core.loader.pactbroker.PactBroker)

Looking at code we find https://github.com/arquillian/arquillian-algeron/blob/master/pact/provider/core/src/main/java/org/arquillian/algeron/pact/provider/core/PactsRetriever.java#L129 as the source of the exception.

When looking at PactBrokerLoader I see that there used to be such a constructor in alpha4 but it disappeared in alpha5. https://github.com/arquillian/arquillian-algeron/blob/1.0.0.Alpha4/provider/pact-broker-loader/src/main/java/org/arquillian/pact/provider/core/loader/pactbroker/PactBrokerLoader.java#L74

The new implementation of PactBrokerLoader certainly shows a nicer way to handle the configuration that the constructor way before. PactsRetriever hasn't been updated to that behavior so far.

bartoszmajsak commented 7 years ago

Thanks for the detailed report and sorry for the trouble. Would you be interested in fixing it in the PR?

scheuchzer commented 7 years ago

I'm having a look at it. But while debugging the code I start to wonder if the PactBroker code ever worked before. At https://github.com/arquillian/arquillian-algeron/blob/master/pact/provider/core/src/main/java/org/arquillian/algeron/pact/provider/core/PactsRetriever.java#L85 there's a filter for .json. The PactBroker doesn't use .json urls. Is there a reason for this filter?

scheuchzer commented 7 years ago

Is there a slack or gitter or whatever channel where I can contact you? I think there's a lot to discuss about the pact integration.

scheuchzer commented 7 years ago

But that's not how the current implementation is working. It gets the urls for the Pacts (without .json) from PactBroker and then PactRetriever drops the urls because of the missing .json.

lordofthejars commented 7 years ago

This is what I am trying to see, if href should contain or not the json. Trying to check the spec

bartoszmajsak commented 7 years ago

Is there a slack or gitter or whatever channel where I can contact you? I think there's a lot to discuss about the pact integration.

We do have (a bit abandoned) IRC #arquillian channel on irc.freenode.net, but I'm there most of the times. Also http://discuss.arquillian.org/ is another place to discuss (not as different to this one though).

Thanks for all your feedback

lordofthejars commented 7 years ago

The truth is that we change the code to make it more generic, and maybe this was the origin of the failure, but also I think I already checked the spec.

lordofthejars commented 7 years ago

On Friday I'll take a look on what it is returning pact broker and how to fix everything

lordofthejars commented 7 years ago

Thank you very much for using Algeron, if you can wait until Friday so we provide a fix this would be awesome, also feel free to provide a PR, we always welcome PRs?

scheuchzer commented 7 years ago

We'd like to use algeron in my company for our wildfly apps. New apps are on spring-boot, so I might not get that much time to contribute large changes. My first impression is, that there's quite some stuff missing in that integration and that it will take quite some time until this becomes production ready. Shall I open some new issues for topics like:

lordofthejars commented 7 years ago

Fixed on #71

PMT87 commented 5 years ago

Is this really fixed?

from pact/provider/core/PactsRetriever

    protected List<Pact> loadContractFiles(List<URI> contracts, String providerName) {
        if (contracts != null) {
            List<URI> contractFiles = contracts.stream()
                .filter(uri -> uri.toString().endsWith(".json"))
                .collect(toList());

            if (contractFiles != null) {
                return contractFiles.stream()
                    .map(URI::toString)
                    .map(PactReader::loadPact)
                    .filter(pact -> pact.getProvider().getName().equals(providerName))
                    .collect(Collectors.toList());
            }
        }
        return new ArrayList<>();
    }

i have contract uris like "https://broker-url/pacts/provider/MY-PROVIDER/consumer/MY-CONSUMER/version/A-VERSION" they will never end with .json and will be sorted out always. Authentications seems also to be a problem. PactReader has an options Map as parameter, also for authentication information. But PactRetriever doesnt hand over any options.

and i only got there with a small fix in PactBrokerLoader

  private List<URI> toUri(List<PactBrokerConsumer> consumerInfos) {
    return consumerInfos.stream()
//        .map(PactBrokerConsumer::getPactBrokerUrl)
        .map(PactBrokerConsumer::getSource)
        .map(URI::create)
        .collect(Collectors.toList());
  }

instead of using the (always the same for all pacts) pactBrokerUrl, we need the source url.