Repository: pions/webrtc
Pull request #527 (2019-03-19 04:54:48)

Change PeerConnection public props to methods
The change is based on the PR #524 since both of these are about turning public properties to methods. Please review and merge the PR 524 first.

Relates to #526
Blocked by PR #524

coveralls: ## Pull Request Test Coverage Report for [Build 2546](https://coveralls.io/builds/22260377)

* **145** of **153**   **(94.77%)**  changed or added relevant lines in **3** files are covered.
* **1** unchanged line in **1** file lost coverage.
* Overall coverage increased (+**0.5%**) to **81.309%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [datachannel.go](https://coveralls.io/builds/22260377/source?filename=datachannel.go#L349) | 93 | 96 | 96.88%
| [peerconnection.go](https://coveralls.io/builds/22260377/source?filename=peerconnection.go#L607) | 46 | 51 | 90.2%
<!-- | **Total:** | **145** | **153** | **94.77%** | -->

|  Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [datachannel.go](https://coveralls.io/builds/22260377/source?filename=datachannel.go#L115) | 1 | 76.61% |
<!-- | **Total:** | **1** |  | -->

|  Totals | [![Coverage Status](https://coveralls.io/builds/22260377/badge)](https://coveralls.io/builds/22260377) |
| :-- | --: |
| Change from base [Build 2539](https://coveralls.io/builds/22226136): |  0.5% |
| Covered Lines: | 4211 |
| Relevant Lines: | 5179 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/peerconnection.go b/peerconnection.go
index f769fad9..3527c389 100644
--- a/peerconnection.go
+++ b/peerconnection.go
@@ -32,46 +32,14 @@ type PeerConnection struct {
 
 	configuration Configuration
 
-	// CurrentLocalDescription represents the local description that was
-	// successfully negotiated the last time the PeerConnection transitioned
-	// into the stable state plus any local candidates that have been generated
-	// by the ICEAgent since the offer or answer was created.
-	CurrentLocalDescription *SessionDescription
-
-	// PendingLocalDescription represents a local description that is in the
-	// process of being negotiated plus any local candidates that have been
-	// generated by the ICEAgent since the offer or answer was created. If the
-	// PeerConnection is in the stable state, the value is null.
-	PendingLocalDescription *SessionDescription
-
-	// CurrentRemoteDescription represents the last remote description that was
-	// successfully negotiated the last time the PeerConnection transitioned
-	// into the stable state plus any remote candidates that have been supplied
-	// via AddICECandidate() since the offer or answer was created.
-	CurrentRemoteDescription *SessionDescription
-
-	// PendingRemoteDescription represents a remote description that is in the
-	// process of being negotiated, complete with any remote candidates that
-	// have been supplied via AddICECandidate() since the offer or answer was
-	// created. If the PeerConnection is in the stable state, the value is
-	// null.
-	PendingRemoteDescription *SessionDescription
-
-	// SignalingState attribute returns the signaling state of the
-	// PeerConnection instance.
-	SignalingState SignalingState
-
-	// ICEGatheringState attribute returns the ICE gathering state of the
-	// PeerConnection instance.
-	ICEGatheringState ICEGatheringState // FIXME NOT-USED
-
-	// ICEConnectionState attribute returns the ICE connection state of the
-	// PeerConnection instance.
-	iceConnectionState ICEConnectionState
-
-	// ConnectionState attribute returns the connection state of the
-	// PeerConnection instance.
-	ConnectionState PeerConnectionState
+	currentLocalDescription  *SessionDescription
+	pendingLocalDescription  *SessionDescription
+	currentRemoteDescription *SessionDescription
+	pendingRemoteDescription *SessionDescription
+	signalingState           SignalingState
+	iceGatheringState        ICEGatheringState // FIXME NOT-USED
+	iceConnectionState       ICEConnectionState
+	connectionState          PeerConnectionState
 
 	idpLoginURL *string
 
@@ -134,10 +102,10 @@ func (api *API) NewPeerConnection(configuration Configuration) (*PeerConnection,
 		negotiationNeeded:  false,
 		lastOffer:          "",
 		lastAnswer:         "",
-		SignalingState:     SignalingStateStable,
+		signalingState:     SignalingStateStable,
 		iceConnectionState: ICEConnectionStateNew,
-		ICEGatheringState:  ICEGatheringStateNew,
-		ConnectionState:    PeerConnectionStateNew,
+		iceGatheringState:  ICEGatheringStateNew,
+		connectionState:    PeerConnectionStateNew,
 		dataChannels:       make(map[uint16]*DataChannel),
 
 		api: api,
@@ -600,7 +568,7 @@ func (pc *PeerConnection) setDescription(sd *SessionDescription, op stateChangeO
 		return &rtcerr.InvalidStateError{Err: ErrConnectionClosed}
 	}
 
-	cur := pc.SignalingState
+	cur := pc.signalingState
 	setLocal := stateChangeOpSetLocal
 	setRemote := stateChangeOpSetRemote
 	newSDPDoesNotMatchOffer := &rtcerr.InvalidModificationError{Err: fmt.Errorf("new sdp does not match previous offer")}
@@ -618,7 +586,7 @@ func (pc *PeerConnection) setDescription(sd *SessionDescription, op stateChangeO
 			}
 			nextState, err = checkNextSignalingState(cur, SignalingStateHaveLocalOffer, setLocal, sd.Type)
 			if err == nil {
-				pc.PendingLocalDescription = sd
+				pc.pendingLocalDescription = sd
 			}
 		// have-remote-offer->SetLocal(answer)->stable
 		// have-local-pranswer->SetLocal(answer)->stable
@@ -628,15 +596,15 @@ func (pc *PeerConnection) setDescription(sd *SessionDescription, op stateChangeO
 			}
 			nextState, err = checkNextSignalingState(cur, SignalingStateStable, setLocal, sd.Type)
 			if err == nil {
-				pc.CurrentLocalDescription = sd
-				pc.CurrentRemoteDescription = pc.PendingRemoteDescription
-				pc.PendingRemoteDescription = nil
-				pc.PendingLocalDescription = nil
+				pc.currentLocalDescription = sd
+				pc.currentRemoteDescription = pc.pendingRemoteDescription
+				pc.pendingRemoteDescription = nil
+				pc.pendingLocalDescription = nil
 			}
 		case SDPTypeRollback:
 			nextState, err = checkNextSignalingState(cur, SignalingStateStable, setLocal, sd.Type)
 			if err == nil {
-				pc.PendingLocalDescription = nil
+				pc.pendingLocalDescription = nil
 			}
 		// have-remote-offer->SetLocal(pranswer)->have-local-pranswer
 		case SDPTypePranswer:
@@ -645,7 +613,7 @@ func (pc *PeerConnection) setDescription(sd *SessionDescription, op stateChangeO
 			}
 			nextState, err = checkNextSignalingState(cur, SignalingStateHaveLocalPranswer, setLocal, sd.Type)
 			if err == nil {
-				pc.PendingLocalDescription = sd
+				pc.pendingLocalDescription = sd
 			}
 		default:
 			return &rtcerr.OperationError{Err: fmt.Errorf("invalid state change op: %s(%s)", op, sd.Type)}
@@ -656,28 +624,28 @@ func (pc *PeerConnection) setDescription(sd *SessionDescription, op stateChangeO
 		case SDPTypeOffer:
 			nextState, err = checkNextSignalingState(cur, SignalingStateHaveRemoteOffer, setRemote, sd.Type)
 			if err == nil {
-				pc.PendingRemoteDescription = sd
+				pc.pendingRemoteDescription = sd
 			}
 		// have-local-offer->SetRemote(answer)->stable
 		// have-remote-pranswer->SetRemote(answer)->stable
 		case SDPTypeAnswer:
 			nextState, err = checkNextSignalingState(cur, SignalingStateStable, setRemote, sd.Type)
 			if err == nil {
-				pc.CurrentRemoteDescription = sd
-				pc.CurrentLocalDescription = pc.PendingLocalDescription
-				pc.PendingRemoteDescription = nil
-				pc.PendingLocalDescription = nil
+				pc.currentRemoteDescription = sd
+				pc.currentLocalDescription = pc.pendingLocalDescription
+				pc.pendingRemoteDescription = nil
+				pc.pendingLocalDescription = nil
 			}
 		case SDPTypeRollback:
 			nextState, err = checkNextSignalingState(cur, SignalingStateStable, setRemote, sd.Type)
 			if err == nil {
-				pc.PendingRemoteDescription = nil
+				pc.pendingRemoteDescription = nil
 			}
 		// have-local-offer->SetRemote(pranswer)->have-remote-pranswer
 		case SDPTypePranswer:
 			nextState, err = checkNextSignalingState(cur, SignalingStateHaveRemotePranswer, setRemote, sd.Type)
 			if err == nil {
-				pc.PendingRemoteDescription = sd
+				pc.pendingRemoteDescription = sd
 			}
 		default:
 			return &rtcerr.OperationError{Err: fmt.Errorf("invalid state change op: %s(%s)", op, sd.Type)}
@@ -687,7 +655,7 @@ func (pc *PeerConnection) setDescription(sd *SessionDescription, op stateChangeO
 	}
 
 	if err == nil {
-		pc.SignalingState = nextState
+		pc.signalingState = nextState
 		pc.onSignalingStateChange(nextState)
 	}
 	return err
@@ -722,21 +690,21 @@ func (pc *PeerConnection) SetLocalDescription(desc SessionDescription) error {
 	return pc.setDescription(&desc, stateChangeOpSetLocal)
 }
 
-// LocalDescription returns PendingLocalDescription if it is not null and
-// otherwise it returns CurrentLocalDescription. This property is used to
+// LocalDescription returns pendingLocalDescription if it is not null and
+// otherwise it returns currentLocalDescription. This property is used to
 // determine if setLocalDescription has already been called.
 // https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-localdescription
 func (pc *PeerConnection) LocalDescription() *SessionDescription {
-	if pc.PendingLocalDescription != nil {
-		return pc.PendingLocalDescription
+	if pc.pendingLocalDescription != nil {
+		return pc.pendingLocalDescription
 	}
-	return pc.CurrentLocalDescription
+	return pc.currentLocalDescription
 }
 
 // SetRemoteDescription sets the SessionDescription of the remote peer
 func (pc *PeerConnection) SetRemoteDescription(desc SessionDescription) error {
 	// FIXME: Remove this when renegotiation is supported
-	if pc.CurrentRemoteDescription != nil {
+	if pc.currentRemoteDescription != nil {
 		return fmt.Errorf("remoteDescription is already defined, SetRemoteDescription can only be called once")
 	}
 	if pc.isClosed {
@@ -954,7 +922,7 @@ func (pc *PeerConnection) openSRTP() {
 			pc.mu.RLock()
 			defer pc.mu.RUnlock()
 
-			sdpCodec, err := pc.CurrentLocalDescription.parsed.GetCodecForPayloadType(receiver.Track().PayloadType())
+			sdpCodec, err := pc.currentLocalDescription.parsed.GetCodecForPayloadType(receiver.Track().PayloadType())
 			if err != nil {
 				pcLog.Warnf("no codec could be found in RemoteDescription for payloadType %d", receiver.Track().PayloadType())
 				return
@@ -1041,15 +1009,15 @@ func (pc *PeerConnection) drainSRTP() {
 	}
 }
 
-// RemoteDescription returns PendingRemoteDescription if it is not null and
-// otherwise it returns CurrentRemoteDescription. This property is used to
+// RemoteDescription returns pendingRemoteDescription if it is not null and
+// otherwise it returns currentRemoteDescription. This property is used to
 // determine if setRemoteDescription has already been called.
 // https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-remotedescription
 func (pc *PeerConnection) RemoteDescription() *SessionDescription {
-	if pc.PendingRemoteDescription != nil {
-		return pc.PendingRemoteDescription
+	if pc.pendingRemoteDescription != nil {
+		return pc.pendingRemoteDescription
 	}
-	return pc.CurrentRemoteDescription
+	return pc.currentRemoteDescription
 }
 
 // AddICECandidate accepts an ICE candidate string and adds it
@@ -1310,14 +1278,14 @@ func (pc *PeerConnection) Close() error {
 	pc.isClosed = true
 
 	// https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-close (step #4)
-	pc.SignalingState = SignalingStateClosed
+	pc.signalingState = SignalingStateClosed
 
 	// https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-close (step #11)
 	// pc.ICEConnectionState = ICEConnectionStateClosed
 	pc.iceStateChange(ice.ConnectionStateClosed) // FIXME REMOVE
 
 	// https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-close (step #12)
-	pc.ConnectionState = PeerConnectionStateClosed
+	pc.connectionState = PeerConnectionStateClosed
 
 	// Try closing everything and collect the errors
 	var closeErrs []error
@@ -1496,3 +1464,55 @@ func (pc *PeerConnection) newRTPTransceiver(
 	pc.rtpTransceivers = append(pc.rtpTransceivers, t)
 	return t
 }
+
+// CurrentLocalDescription represents the local description that was
+// successfully negotiated the last time the PeerConnection transitioned
+// into the stable state plus any local candidates that have been generated
+// by the ICEAgent since the offer or answer was created.
+func (pc *PeerConnection) CurrentLocalDescription() *SessionDescription {
+	return pc.currentLocalDescription
+}
+
+// PendingLocalDescription represents a local description that is in the
+// process of being negotiated plus any local candidates that have been
+// generated by the ICEAgent since the offer or answer was created. If the
+// PeerConnection is in the stable state, the value is null.
+func (pc *PeerConnection) PendingLocalDescription() *SessionDescription {
+	return pc.pendingLocalDescription
+}
+
+// CurrentRemoteDescription represents the last remote description that was
+// successfully negotiated the last time the PeerConnection transitioned
+// into the stable state plus any remote candidates that have been supplied
+// via AddICECandidate() since the offer or answer was created.
+func (pc *PeerConnection) CurrentRemoteDescription() *SessionDescription {
+	return pc.currentRemoteDescription
+}
+
+// PendingRemoteDescription represents a remote description that is in the
+// process of being negotiated, complete with any remote candidates that
+// have been supplied via AddICECandidate() since the offer or answer was
+// created. If the PeerConnection is in the stable state, the value is
+// null.
+func (pc *PeerConnection) PendingRemoteDescription() *SessionDescription {
+	return pc.pendingRemoteDescription
+}
+
+// SignalingState attribute returns the signaling state of the
+// PeerConnection instance.
+func (pc *PeerConnection) SignalingState() SignalingState {
+	return pc.signalingState
+}
+
+// ICEGatheringState attribute returns the ICE gathering state of the
+// PeerConnection instance.
+// FIXME NOT-USED
+func (pc *PeerConnection) ICEGatheringState() ICEGatheringState {
+	return pc.iceGatheringState
+}
+
+// ConnectionState attribute returns the connection state of the
+// PeerConnection instance.
+func (pc *PeerConnection) ConnectionState() PeerConnectionState {
+	return pc.connectionState
+}


Test patch:
diff --git a/peerconnection_go_test.go b/peerconnection_go_test.go
index e2a3bbff..ad007e9c 100644
--- a/peerconnection_go_test.go
+++ b/peerconnection_go_test.go
@@ -357,3 +357,25 @@ func TestPeerConnection_ShutdownNoDTLS(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+func TestPeerConnection_PeropertyGetters(t *testing.T) {
+	pc := &PeerConnection{
+		currentLocalDescription:  &SessionDescription{},
+		pendingLocalDescription:  &SessionDescription{},
+		currentRemoteDescription: &SessionDescription{},
+		pendingRemoteDescription: &SessionDescription{},
+		signalingState:           SignalingStateHaveLocalOffer,
+		iceGatheringState:        ICEGatheringStateGathering,
+		iceConnectionState:       ICEConnectionStateChecking,
+		connectionState:          PeerConnectionStateConnecting,
+	}
+
+	assert.Equal(t, pc.currentLocalDescription, pc.CurrentLocalDescription(), "should match")
+	assert.Equal(t, pc.pendingLocalDescription, pc.PendingLocalDescription(), "should match")
+	assert.Equal(t, pc.currentRemoteDescription, pc.CurrentRemoteDescription(), "should match")
+	assert.Equal(t, pc.pendingRemoteDescription, pc.PendingRemoteDescription(), "should match")
+	assert.Equal(t, pc.signalingState, pc.SignalingState(), "should match")
+	assert.Equal(t, pc.iceGatheringState, pc.ICEGatheringState(), "should match")
+	assert.Equal(t, pc.iceConnectionState, pc.ICEConnectionState(), "should match")
+	assert.Equal(t, pc.connectionState, pc.ConnectionState(), "should match")
+}