Skip to content

Removed semaphore from swift snippet #664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 3, 2023
Merged

Removed semaphore from swift snippet #664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ab44f86
[IMPORT-744] removed semaphore from swift snippet
SahilChoudhary22 Jan 30, 2023
ce70fcf
Merge branch 'develop' into feature/remove-semaphore-from-swift-snippet
SahilChoudhary22 Jan 31, 2023
eeaeace
Added workflow trigger on pull request
SahilChoudhary22 Jan 31, 2023
7901957
added semaphore to output response in swift test
SahilChoudhary22 Feb 1, 2023
7312a4a
now using exit instead of semaphore for boilerplates, removed unused …
SahilChoudhary22 Feb 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/integration.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: Test

on: push
on: [push, pull_request]

jobs:
Unit-Tests:
Expand Down
28 changes: 15 additions & 13 deletions codegens/swift/lib/swift.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,11 @@ self = module.exports = {
description: 'Remove white space and additional lines that may affect the server\'s response'
},
{
name: 'Follow redirects',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SahilChoudhary22 any specific reason we removed this option? We might need to block the redirects if its unchecked right? Or you removed as it was not getting used.

cc @VShingala do we not block redirects if this flag is off 🙇‍♂️

Copy link
Contributor Author

@SahilChoudhary22 SahilChoudhary22 Feb 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akshaydeo Yes, we observed that this option was not getting used in the business logic and therefore it being true or false was not making any difference to the output. Also, it was observed that it was not mentioned in the swift test cases (Ref)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akshaydeo Swift by default follows redirection and from our research, there was no easy way to control this behavior from swift using some option. So yeah as it's not possible from Swift language itself, the option was not used.

id: 'followRedirect',
name: 'Include boilerplate',
id: 'includeBoilerplate',
type: 'boolean',
default: true,
description: 'Automatically follow HTTP redirects'
default: false,
description: 'Include class definition and import statements in snippet'
}
];
},
Expand Down Expand Up @@ -286,15 +286,15 @@ self = module.exports = {
throw new Error('Swift-Converter: callback is not valid function');
}
options = sanitizeOptions(options, self.getOptions());
var codeSnippet, indent, trim, timeout, finalUrl, // followRedirect,
var indent, trim, timeout, finalUrl,
codeSnippet = '',
bodySnippet = '',
headerSnippet = '',
requestBody;

indent = options.indentType === 'Tab' ? '\t' : ' ';
indent = indent.repeat(options.indentCount);
timeout = options.requestTimeout;
// followRedirect = options.followRedirect;
trim = options.trimRequestBody;
finalUrl = getUrlStringfromUrlObject(request.url);

Expand Down Expand Up @@ -342,9 +342,10 @@ self = module.exports = {
requestBody = (request.body ? request.body.toJSON() : {});
bodySnippet = parseBody(requestBody, trim, indent);

codeSnippet = 'import Foundation\n';
codeSnippet += '#if canImport(FoundationNetworking)\nimport FoundationNetworking\n#endif\n\n';
codeSnippet += 'var semaphore = DispatchSemaphore (value: 0)\n\n';
if (options.includeBoilerplate) {
codeSnippet += 'import Foundation\n';
codeSnippet += '#if canImport(FoundationNetworking)\nimport FoundationNetworking\n#endif\n\n';
}
if (bodySnippet !== '') {
codeSnippet += `${bodySnippet}\n\n`;
}
Expand Down Expand Up @@ -375,13 +376,14 @@ self = module.exports = {
codeSnippet += '\nlet task = URLSession.shared.dataTask(with: request) { data, response, error in \n';
codeSnippet += `${indent}guard let data = data else {\n`;
codeSnippet += `${indent.repeat(2)}print(String(describing: error))\n`;
codeSnippet += `${indent.repeat(2)}semaphore.signal()\n`;
codeSnippet += `${indent.repeat(2)}return\n`;
codeSnippet += `${indent.repeat(2)}`;
codeSnippet += options.includeBoilerplate ? 'exit(EXIT_SUCCESS)\n' : 'return\n';
codeSnippet += `${indent}}\n`;
codeSnippet += `${indent}print(String(data: data, encoding: .utf8)!)\n`;
codeSnippet += `${indent}semaphore.signal()\n}\n\n`;
codeSnippet += options.includeBoilerplate ? `${indent}exit(EXIT_SUCCESS)\n` : '';
codeSnippet += '}\n\n';
codeSnippet += 'task.resume()\n';
codeSnippet += 'semaphore.wait()\n';
codeSnippet += options.includeBoilerplate ? 'dispatchMain()\n' : '';

return callback(null, codeSnippet);
}
Expand Down
1 change: 1 addition & 0 deletions codegens/swift/test/newman/newman.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ var runNewmanTest = require('../../../../test/codegen/newman/newmanTestUtil').ru

describe('Convert for different types of request', function () {
var options = {
includeBoilerplate: true,
indentType: 'Space',
indentCount: 4
},
Expand Down
1 change: 1 addition & 0 deletions codegens/swift/test/unit/convert.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ describe('Swift Converter', function () {
expect(getOptions()[1]).to.have.property('id', 'indentType');
expect(getOptions()[2]).to.have.property('id', 'requestTimeout');
expect(getOptions()[3]).to.have.property('id', 'trimRequestBody');
expect(getOptions()[4]).to.have.property('id', 'includeBoilerplate');
});
});

Expand Down