Convert Project to Multi Module

[Shah-Parth]

• Converting project to multi module
• adding -SNAPSHOT to the release tag
• improving README

[Shah-Parth]

closes #10

[jeffusan]

My understanding is that provided scope dependencies will need to be explicitly added by programs using this library. Is this really what we want?

[Shah-Parth]

You're right... compiled is probably what we need to resolve the transitive dependencies.

[jeffusan]

Is this commented out because it's optional?

[jeffusan]

I recommend adding a comment/justification for commented out blocks. I prefer just removing them.

[jeffusan]

Are the p12 and truststore intended to be published with the OSS artifact?

[jeffusan]

The *ResponseHandlers seem to have duplicate logic, ie:

    @Override
    public ArrayList<ResponseMetadata> handleResponse(final HttpResponse response)
            throws IOException {
        final StatusLine statusLine = response.getStatusLine();
        final HttpEntity entity = response.getEntity();
        if (statusLine.getStatusCode() >= 300) {
            EntityUtils.consume(entity);
            throw new HttpResponseException(statusLine.getStatusCode(),
                    statusLine.getReasonPhrase());
        }
        return entity == null ? null : handleEntity(entity);
    }

    /**
     * Returns the entity as a body as a String.
     */
    @Override
    public ArrayList<ResponseMetadata> handleEntity(final HttpEntity entity) throws IOException {
        String entityString = EntityUtils.toString(entity);
        Gson gson = new Gson();

        return gson.fromJson(entityString, new TypeToken<ArrayList<ResponseMetadata>>() {
        }.getType());
    }

Why is this duplicated in several classes instead of being just one class?

[jeffusan]

GreyMatterCloseableHttpClient has a code block commented out:

    //TODO: uncomment once {@link GreyMatterDataStreamResponseHandler} is implemented
//    @Deprecated
//    public InputStream execute(final GreyMatterDataStreamRequest request) throws IOException, ClientProtocolException {
//        return execute(request, new GreyMatterDataStreamResponseHandler(), null);
//    }

GreyMatterDataStreamResponseHandler exists, but has a comment to the effect "this needs more thought".

What is the status of GreyMatterDataStreamResponseHandler and the intent going forward? Is it something that still needs to be implemented?

[jeffusan]

I'd prefer to see commented out code removed and replaced with issues.

[Shah-Parth]

GreyMatterCloseableHttpClient has a code block commented out:

    //TODO: uncomment once {@link GreyMatterDataStreamResponseHandler} is implemented
//    @Deprecated
//    public InputStream execute(final GreyMatterDataStreamRequest request) throws IOException, ClientProtocolException {
//        return execute(request, new GreyMatterDataStreamResponseHandler(), null);
//    }

GreyMatterDataStreamResponseHandler exists, but has a comment to the effect "this needs more thought".

What is the status of GreyMatterDataStreamResponseHandler and the intent going forward? Is it something that still needs to be implemented?

Yes, I need someone more familiar to take a look and tell me what I can do to provide users to leverage GreyMatterDataStreamResponseHandler instead of generic HttpResponse. If we can implement the logic inside GreyMatterDataStreamResponseHandler we can customize it to specific gm-data needs with standardized errors.

[Shah-Parth]

The *ResponseHandlers seem to have duplicate logic, ie:

    @Override
    public ArrayList<ResponseMetadata> handleResponse(final HttpResponse response)
            throws IOException {
        final StatusLine statusLine = response.getStatusLine();
        final HttpEntity entity = response.getEntity();
        if (statusLine.getStatusCode() >= 300) {
            EntityUtils.consume(entity);
            throw new HttpResponseException(statusLine.getStatusCode(),
                    statusLine.getReasonPhrase());
        }
        return entity == null ? null : handleEntity(entity);
    }

    /**
     * Returns the entity as a body as a String.
     */
    @Override
    public ArrayList<ResponseMetadata> handleEntity(final HttpEntity entity) throws IOException {
        String entityString = EntityUtils.toString(entity);
        Gson gson = new Gson();

        return gson.fromJson(entityString, new TypeToken<ArrayList<ResponseMetadata>>() {
        }.getType());
    }

Why is this duplicated in several classes instead of being just one class?

All of these classes are there as a placeholder for the initial starting point, we can for sure improve these classes and handle specific errors based on the particular endpoints.
Yes, there is a lot of redundant code, but at the same time this is just an initial prototype of the SDK and it for sure needs more contribution. I don't think this is too critical at this point that it needs to be resolved in this PR.

[Shah-Parth]

Can someone please approve the PR so I can merge it.