Bug 950660

Summary:	RFE: [as7] Use Commons HttpClient in ASConnection instead of JDK's HttpUrlConnection
Product:	[JBoss] JBoss Operations Network	Reporter:	Thomas Segismont <tsegismo>
Component:	Plugin -- JBoss EAP 6	Assignee:	Thomas Segismont <tsegismo>
Status:	CLOSED NEXTRELEASE	QA Contact:	Mike Foley <mfoley>
Severity:	high	Docs Contact:
Priority:	urgent
Version:	JON 3.1.2	CC:	hrupp, loleary
Target Milestone:	---
Target Release:	JON 3.1.3
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:	929200
Clones:	951738 (view as bug list)		Environment:
Last Closed:	2013-09-06 02:25:34 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:	929200
Bug Blocks:	951736, 951738

Description Thomas Segismont 2013-04-10 15:11:23 UTC

+++ This bug was initially created as a clone of Bug #929200 +++

+++ This bug was initially created as a clone of Bug #929198 +++

In AS7 plugin, ASConnection class currently uses JDK's HttpUrlConnection to communicate with an AS7 instance.

This class should be updated to use Commons HttpClient, like ASUploadConnection class does since release 4.6 (see BZ887320)

Commons HttpClient gives us more control on the communication process and has proven to be more reliable.

--- Additional comment from Thomas Segismont on 2013-03-29 14:28:38 CET ---

See BZ918677 for the type of problems we have with JDK's HttpUrlConnection

--- Additional comment from Charles Crouch on 2013-03-29 15:34:29 CET ---

Since this seems to be related more to fixing a bug than adding a feature I've removed the FutureFeature keyword. I've also unset the Target Release field to make sure this gets triaged.

A change in the http library used by the AS7 plugin will require a full regression test run, since http communication is used so widely to talk to AS7

--- Additional comment from Thomas Segismont on 2013-04-02 10:57:36 CEST ---

BZ918677 has been partially fixed (only works on some platforms). Moving ASConnection to Commons HttpClient will allow to get rid of the problem on all platforms.

ASConnection is the core communication component of AS7 plugin. Changing its implementation will involve extensive non regression testing from QE before the next JON release. But as ASUploadConnection has already been moved to commons HttpClient due to BZ887320, it has to be done anyway.

As for the benefits of this change:
* cohesion between ASConnection and ASUploadConnection implementations
* easier debugging as JDK's HttpUrlConnection is closed source (at least in Oracle's VM)
* Commons HttpClient gives the developer more control on the communication process (when/how open or close connections, configurable pooling, unshared authentication)

--- Additional comment from Thomas Segismont on 2013-04-02 11:56:28 CEST ---

And for the risks.

No instance of HttpUrlConnection is shared outside ASConnection. So components in AS7 plugin only rely on ASConnection contract.

Consequently, while ASConnection is the core communication component of AS7 plugin, the risk of regression is very low.

--- Additional comment from Thomas Segismont on 2013-04-08 23:42:52 CEST ---

Fixed in master - 21ec5e2

AS7 plugin itests passed succesfully

Beware that this touches the core connection part (ASConnection class) and hence requires a full non regresssion testing of AS7 plugin.

--- Additional comment from Thomas Segismont on 2013-04-09 12:08:13 CEST ---

Reworked to increased delay in AS7 itests to discover all resources
master - 3b18013

Comment 1 Thomas Segismont 2013-04-10 15:17:22 UTC

Changes reported from master to release/jon3.1.x - 89d941d

All unit and integration tests pass on my box and on Jenkins:
https://jenkins.mw.lab.eng.bos.redhat.com/hudson/view/RHQ/job/rhq-master/org.rhq$rhq-jboss-as-7-plugin/

Comment 2 Larry O'Leary 2013-04-17 14:57:32 UTC

Moving back to ON_DEV as this fix seems to introduce regressions. See comment in downstream bug 951738#c4.

Comment 3 Larry O'Leary 2013-04-18 14:07:30 UTC

As per comment on downstream bug 951738#c8:

~~~~
Thomas Segismont 2013-04-18 05:11:09 EDT

Pushed commit 1c01db5 to release/jon3.1.x branch (cherry-picked from master - 915ab3d) as integration tests passed:
https://jenkins.mw.lab.eng.bos.redhat.com/hudson/view/RHQ/job/rhq-master/org.rhq$rhq-jboss-as-7-plugin/2168/
~~~~

Moving to MODIFIED for 3.1.3.

Comment 4 Larry O'Leary 2013-09-06 02:25:34 UTC

Closing as there will not be a 3.1.3 release. This is being tracked for 3.2 in the 'depends on' field.