artifactory-client-java icon indicating copy to clipboard operation
artifactory-client-java copied to clipboard

Return a List<String> for quick search, not a Map<String, List<Map<String, String>>>

Open asarkar opened this issue 8 years ago • 4 comments

The return type for Artifactory.restCall for GET /api/search/artifact?repos=libs-release-local&name=sources turns out to be Map<String, List<Map<String, String>>>. Quite frankly, the return type appears to be the choice of a 3 year old when asked to point at different classes in the JDK. The outer Map will only ever have one key, namely results, and all the inner Maps will ever have a single entry uri -> something, so I can't figure out the reasoning behind this choice of return type.

GET /api/search/artifact?repos=libs-release-local&name=sources
{
"results": [
    {
        "uri": "http://localhost:8081/artifactory/api/storage/libs-release-local/org/acme/artifact/1.0/artifact-1.0-sources.jar"
    },{
        "uri": "http://localhost:8081/artifactory/api/storage/libs-release-local/org/acme/artifactB/1.0/artifactB-1.0-sources.jar"
    }
]
}

Instead, a simple Collection<String> of all URIs found will suffice. If a JSON object must be returned instead of an array, a simple POJO like the following will be welcomed by the clients of the SDK.

public class SearchResponse {
    private List<String> results;
    // getters and setters
}

In the case of POJO, the response will obviously change to:

GET /api/search/artifact?repos=libs-release-local&name=sources
{
"results": [
    "http://localhost:8081/artifactory/api/storage/libs-release-local/org/acme/artifact/1.0/artifact-1.0-sources.jar",
    "http://localhost:8081/artifactory/api/storage/libs-release-local/org/acme/artifactB/1.0/artifactB-1.0-sources.jar"
]
}

asarkar avatar Jun 29 '17 04:06 asarkar

Hi @asarkar, Can you come up with a generic solution that will work with any restCall request? Also, will you be willing to contribute the solution through a pull request?

eyalbe4 avatar Jun 29 '17 04:06 eyalbe4

@eyalbe4

Can you come up with a generic solution that will work with any restCall request?

I'm not sure why the response has to be generic. If you consider the client's' point of view, they are unlikely to make every kind of restCall that can ever exist. For one or 2 calls, strongly typed classes are always preferred over generic solutions. that said, if you show me a sample response for each search, I'll try to come up with a strongly typed object.

will you be willing to contribute the solution through a pull request?

I'm willing to contribute if we reach an agreement. ~~I don't agree with a need for a generic return type for the reasons explained above.~~

asarkar avatar Jun 29 '17 05:06 asarkar

@asarkar, The java-client does support specific (non generic) response types for other APIs. Actually, the restCall API was created to provide a quick workaround for REST APIs that are hadn't been added to the java-client yet. Why not just add a new API (just like all the other APIs except for the restCall) with a compatible response class?

eyalbe4 avatar Jun 29 '17 06:06 eyalbe4

@eyalbe4 Following your comment, I was able to replace the restCall with the following:

artifactory.searches()
    .repositories(artifactoryProperties.getRepo())
    .artifactsByName(artifactoryProperties.getSearchTerm())
    .doSearch()

I think if a return type class is added to restCall, then clients can supply their own response classes. Although, because the Jackson ObjectMapper is not available to them, serialization/deserialization options may be limited.

asarkar avatar Jun 29 '17 09:06 asarkar