[WIP]Access http response #41

lightsprint09 · 2017-06-13T14:29:15Z

Fixes issues: #49 , #39

New feature:
When doing a network request, you may want to access the raw HTTPResponse. In our current implementation is this not possible. This opens up the API to access the HTTPResponse when needed.

Make sure to check all boxes before merging

Method/Class documentation
README.md documentation
Unit tests for new features/regressions

houndci-bot · 2017-06-13T14:29:27Z

Tests/DBNetworkStackTests/RetryTaskTest.swift

@@ -27,6 +27,10 @@ import Foundation
 import XCTest
 @testable import DBNetworkStack

+extension HTTPURLResponse {
+    static let defaultTestResponse = HTTPURLResponse(url: URL(string: "bahn.de")!, mimeType: nil, expectedContentLength: 1, textEncodingName: nil)


Force Unwrapping Violation: Force unwrapping should be avoided. (force_unwrapping)

dennispost · 2017-07-06T16:01:31Z

@lightsprint09 I'm missing a description of the PR goals

dennispost · 2017-07-06T16:03:15Z

Source/NetworkServiceMock.swift

@@ -84,9 +84,9 @@ public class NetworkServiceMock: NetworkServiceProviding {
    /// - Parameters:
    ///   - data: the mock response from the server. `Data()` by default
    ///   - count: the count how often the response gets triggerd. 1 by default


Missing httpResponse parameter doc

dennispost · 2017-07-06T16:05:01Z

Source/NetworkServiceMock.swift

@@ -98,9 +98,9 @@ public class NetworkServiceMock: NetworkServiceProviding {
    /// - Parameters:
    ///   - data: the mock response from the server. `Data()` by default
    ///   - count: the count how often the response gets triggerd. 1 by default


Missing httpResponse parameter doc

dennispost · 2017-07-06T16:06:32Z

Source/RetryNetworkService.swift

@@ -61,15 +61,15 @@ public final class RetryNetworkService: NetworkServiceProviding {
    }

    @discardableResult


Missing function documentation

This is a protocol implementation. So the method is documented by the protocol

codecov · 2017-07-07T05:46:47Z

Codecov Report

Merging #41 into develop will increase coverage by 0.86%.
The diff coverage is 93.1%.

@@             Coverage Diff             @@
##           develop      #41      +/-   ##
===========================================
+ Coverage     86.3%   87.16%   +0.86%     
===========================================
  Files           17       17              
  Lines          292      296       +4     
===========================================
+ Hits           252      258       +6     
+ Misses          40       38       -2

Impacted Files	Coverage Δ
Source/RetryNetworkTask.swift	`88.23% <ø> (ø)`	⬆️
Source/URLRequestConvertible.swift	`82.6% <0%> (ø)`	⬆️
Source/RetryNetworkService.swift	`89.47% <100%> (+0.58%)`	⬆️
Source/NetworkServiceMock.swift	`87.5% <100%> (ø)`	⬆️
Source/NetworkService.swift	`100% <100%> (ø)`	⬆️
Source/ModifyRequestNetworkService.swift	`94.11% <100%> (ø)`	⬆️
Source/NetworkServiceProviding.swift	`100% <100%> (ø)`	⬆️
Source/NetworkResponseProcessor.swift	`96.96% <88.88%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c727a7c...0c4ebda. Read the comment docs.

houndci-bot · 2017-07-21T09:26:32Z

Tests/DBNetworkStackTests/DefaultMocks.swift

+import Foundation
+
+extension URL {
+    static let defaultMock: URL = URL(string: "bahn.de")!


Force Unwrapping Violation: Force unwrapping should be avoided. (force_unwrapping)

houndci-bot · 2017-07-21T09:26:32Z

Tests/DBNetworkStackTests/DefaultMocks.swift

+import Foundation
+
+extension URL {
+    static let defaultMock: URL = URL(string: "bahn.de")!


Force Unwrapping Violation: Force unwrapping should be avoided. (force_unwrapping)

houndci-bot · 2017-07-21T09:26:32Z

Tests/DBNetworkStackTests/DefaultMocks.swift

+import Foundation
+
+extension URL {
+    static let defaultMock: URL = URL(string: "bahn.de")!


Force Unwrapping Violation: Force unwrapping should be avoided. (force_unwrapping)

houndci-bot · 2017-07-25T10:50:19Z

Source/RetryNetworkService.swift

        }, dispatchRetry: { [weak self] disptachTime, block in
            self?.dispatchRetry(disptachTime, block)
        })
-        retryTask.originalTask = networkService.request(resource, onCompletion: onCompletion, onError: retryTask.createOnError())
+        retryTask.originalTask = networkService.request(queue: queue, resource: resource, onCompletionWithResponse: onCompletionWithResponse, onError: retryTask.createOnError())


Line Length Violation: Line should be 160 characters or less: currently 177 characters (line_length)

houndci-bot · 2017-07-25T10:50:19Z

Source/RetryNetworkService.swift

        }, dispatchRetry: { [weak self] disptachTime, block in
            self?.dispatchRetry(disptachTime, block)
        })
-        retryTask.originalTask = networkService.request(resource, onCompletion: onCompletion, onError: retryTask.createOnError())
+        retryTask.originalTask = networkService.request(queue: queue, resource: resource, onCompletionWithResponse: onCompletionWithResponse, onError: retryTask.createOnError())


Line Length Violation: Line should be 160 characters or less: currently 177 characters (line_length)

houndci-bot · 2017-07-25T10:50:19Z

Source/RetryNetworkService.swift

        }, dispatchRetry: { [weak self] disptachTime, block in
            self?.dispatchRetry(disptachTime, block)
        })
-        retryTask.originalTask = networkService.request(resource, onCompletion: onCompletion, onError: retryTask.createOnError())
+        retryTask.originalTask = networkService.request(queue: queue, resource: resource, onCompletionWithResponse: onCompletionWithResponse, onError: retryTask.createOnError())


Line Length Violation: Line should be 160 characters or less: currently 177 characters (line_length)

ch-one

Aside from the typo, everything looks fine 👍

ch-one · 2017-07-25T10:45:31Z

Source/NetworkResponseProcessor.swift

-            }
-        } catch {
+        } catch let parsingError {
+            let dbBetworkError: DBNetworkStackError! = parsingError as? DBNetworkStackError


dbBetworkError => dbNetworkError

🦇 -Work??

dennispost · 2017-07-31T11:53:02Z

Source/NetworkServiceProviding.swift

@@ -33,11 +33,11 @@ import Dispatch
 */
 public protocol NetworkServiceProviding {
    /**
-     Fetches a resource asynchrony from remote location
+     Fetches a resource asynchrony from remote location.


Do you mean 'asynchronously'?

dennispost · 2017-07-31T11:53:35Z

Source/NetworkServiceProviding.swift

@@ -49,9 +49,20 @@ public protocol NetworkServiceProviding {

 public extension NetworkServiceProviding {
    /**
-     Fetches a resource asynchrony from remote location
+     Fetches a resource asynchrony from remote location. Completion and Error block will be called on the main thread.


dennispost · 2017-07-31T11:55:26Z

Source/NetworkServiceProviding.swift

@@ -49,9 +49,20 @@ public protocol NetworkServiceProviding {

 public extension NetworkServiceProviding {
    /**
-     Fetches a resource asynchrony from remote location
+     Fetches a resource asynchrony from remote location. Completion and Error block will be called on the main thread.
+


Demo code should be part of Playground

lightsprint09 added 4 commits June 2, 2017 08:18

Support for getting httpResponse when needed

526bc1d

test new httpResponse

e3572f4

Silence swift 3.1 print warning

1d216e5

handle nil response

2391712

lightsprint09 requested a review from ch-one June 13, 2017 14:29

houndci-bot reviewed Jun 13, 2017

View reviewed changes

lightsprint09 mentioned this pull request Jun 28, 2017

Accessing information from response headers #39

Closed

dennispost requested changes Jul 6, 2017

View reviewed changes

Merge branch 'develop' into feature/access-http-response

ca0c727

lightsprint09 added 2 commits July 7, 2017 07:46

Adds documentation for missing httpResponse parameter

5be9019

Refactors ResponseProcessing

8f4e591

houndci-bot reviewed Jul 21, 2017

View reviewed changes

lightsprint09 added 4 commits July 21, 2017 11:49

Improves public declaration

1cf0118

Fixes naming confusion

ba8e8ac

rename 2

90db418

Implements new naming into all services/ tets

2522885

houndci-bot reviewed Jul 25, 2017

View reviewed changes

Fixes swift lint issues

96401dc

ch-one approved these changes Jul 25, 2017

View reviewed changes

lightsprint09 added 4 commits July 25, 2017 12:58

Fixes typo

77954e6

Improvements

fc0eafd

Adds deprecation warnings for resource protocols

f8d4ad9

Documents new httpresponse access

1d01069

dennispost requested changes Jul 31, 2017

View reviewed changes

lightsprint09 and others added 2 commits July 31, 2017 15:53

fixes typos in docu

77888af

Merge branch 'develop' into feature/access-http-response

0c4ebda

lightsprint09 merged commit 0d18f5b into develop Aug 1, 2017

lightsprint09 deleted the feature/access-http-response branch August 1, 2017 06:07

lightsprint09 mentioned this pull request Aug 1, 2017

Refactor NetworkResponseProcessing #49

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP]Access http response #41

[WIP]Access http response #41

lightsprint09 commented Jun 13, 2017 •

edited

houndci-bot Jun 13, 2017

dennispost commented Jul 6, 2017

dennispost Jul 6, 2017

dennispost Jul 6, 2017

dennispost Jul 6, 2017

lightsprint09 Jul 7, 2017

codecov bot commented Jul 7, 2017 •

edited

houndci-bot Jul 21, 2017

houndci-bot Jul 21, 2017

houndci-bot Jul 21, 2017

houndci-bot Jul 25, 2017

houndci-bot Jul 25, 2017

houndci-bot Jul 25, 2017

ch-one left a comment

ch-one Jul 25, 2017

lightsprint09 Jul 25, 2017

dennispost Jul 31, 2017

dennispost Jul 31, 2017

dennispost Jul 31, 2017

		@@ -61,15 +61,15 @@ public final class RetryNetworkService: NetworkServiceProviding {
		}

		@discardableResult

[WIP]Access http response #41

[WIP]Access http response #41

Conversation

lightsprint09 commented Jun 13, 2017 • edited

Make sure to check all boxes before merging

Choose a reason for hiding this comment

dennispost commented Jul 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 7, 2017 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ch-one left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lightsprint09 commented Jun 13, 2017 •

edited

codecov bot commented Jul 7, 2017 •

edited