-
Notifications
You must be signed in to change notification settings - Fork 63
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.
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
Dynamically generated vcf content #420
Merged
Merged
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
1dc7529
implementation of RemoteFile from a String added - this allows user t…
piotr-gawron f11b82a
lint issues
piotr-gawron cdd8226
lint fix
piotr-gawron c288d42
lint issue
piotr-gawron b7e0d6a
armish code review suggestions
piotr-gawron 95f6810
merge issues :)
piotr-gawron File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
/** | ||
* AbstractFile is an abstract representation of a file. There are two implementation: | ||
* 1. RemoteFile - representation of a file on a remote server which can be | ||
* fetched in chunks, e.g. using a Range request. | ||
* 2. LocalStringFile is a representation of a file that was created from input string. | ||
* Used for testing and small input files. | ||
* @flow | ||
*/ | ||
'use strict'; | ||
|
||
//import Q from 'q'; | ||
|
||
class AbstractFile { | ||
constructor() { | ||
//how to prevent instantation of this class??? | ||
//this code doesn't pass npm run flow | ||
// if (new.target === AbstractFile) { | ||
// throw new TypeError("Cannot construct AbstractFile instances directly"); | ||
// } | ||
} | ||
|
||
getBytes(start: number, length: number):Object {//: Q.Promise<ArrayBuffer> { | ||
throw new TypeError("Method getBytes is not implemented"); | ||
} | ||
|
||
// Read the entire file -- not recommended for large files! | ||
getAll():Object {//: Q.Promise<ArrayBuffer> { | ||
throw new TypeError("Method getAll is not implemented"); | ||
} | ||
|
||
// Reads the entire file as a string (not an ArrayBuffer). | ||
// This does not use the cache. | ||
getAllString():Object {//: Q.Promise<string> { | ||
throw new TypeError("Method getAllString is not implemented"); | ||
} | ||
|
||
// Returns a promise for the number of bytes in the remote file. | ||
getSize():Object {//: Q.Promise<number> { | ||
throw new TypeError("Method getSize is not implemented"); | ||
} | ||
} | ||
|
||
module.exports = AbstractFile; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
/** | ||
* LocalStringFile is a representation of a file that was created from input string. Used for testing and small input files. | ||
* @flow | ||
*/ | ||
'use strict'; | ||
|
||
import Q from 'q'; | ||
import AbstractFile from './AbstractFile'; | ||
|
||
class LocalStringFile extends AbstractFile { | ||
fileLength: number; | ||
content: string; //content of this "File" | ||
buffer: ArrayBuffer; | ||
|
||
constructor(content: string) { | ||
super(); | ||
this.fileLength = content.length; | ||
this.buffer = new ArrayBuffer(content.length); // 2 bytes for each char | ||
this.content = content; | ||
|
||
var bufView = new Uint8Array(this.buffer); | ||
for (var i=0; i < this.fileLength; i++) { | ||
bufView[i] = content.charCodeAt(i); | ||
} | ||
} | ||
|
||
getBytes(start: number, length: number): Q.Promise<ArrayBuffer> { | ||
if (length < 0) { | ||
return Q.reject(`Requested <0 bytes (${length})`); | ||
} | ||
|
||
// If the remote file length is known, clamp the request to fit within it. | ||
var stop = start + length - 1; | ||
if (this.fileLength != -1) { | ||
stop = Math.min(this.fileLength - 1, stop); | ||
} | ||
|
||
// First check the cache. | ||
var buf = this.getFromCache(start, stop); | ||
return Q.when(buf); | ||
} | ||
|
||
// Read the entire file -- not recommended for large files! | ||
getAll(): Q.Promise<ArrayBuffer> { | ||
var buf = this.getFromCache(0, this.fileLength - 1); | ||
return Q.when(buf); | ||
} | ||
|
||
// Reads the entire file as a string (not an ArrayBuffer). | ||
// This does not use the cache. | ||
getAllString(): Q.Promise<string> { | ||
return Q.when(this.content); | ||
} | ||
|
||
// Returns a promise for the number of bytes in the remote file. | ||
getSize(): Q.Promise<number> { | ||
return Q.when(this.fileLength); | ||
} | ||
|
||
getFromCache(start: number, stop: number): ?ArrayBuffer { | ||
return this.buffer.slice(start, stop + 1); | ||
} | ||
|
||
} | ||
|
||
module.exports = LocalStringFile; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ import Q from 'q'; | |
|
||
import ContigInterval from '../ContigInterval'; | ||
import RemoteFile from '../RemoteFile'; | ||
import LocalStringFile from '../LocalStringFile'; | ||
import VcfFile from '../data/vcf'; | ||
|
||
export type VcfDataSource = { | ||
|
@@ -91,13 +92,27 @@ function createFromVcfFile(remoteSource: VcfFile): VcfDataSource { | |
return o; | ||
} | ||
|
||
function create(data: {url:string}): VcfDataSource { | ||
function create(data: Object): VcfDataSource { | ||
var url = data.url; | ||
if (!url) { | ||
throw new Error(`Missing URL from track: ${JSON.stringify(data)}`); | ||
var content = data.content; | ||
if (url!==null && url!== undefined) { | ||
return createFromVcfFile(new VcfFile(new RemoteFile(url))); | ||
} | ||
if (content!==null && content!== undefined) { | ||
return createFromVcfFile(new VcfFile(new LocalStringFile(content))); | ||
} | ||
throw new Error(`Missing URL from track: ${JSON.stringify(data)}`); | ||
} | ||
|
||
return createFromVcfFile(new VcfFile(new RemoteFile(url))); | ||
function create(data: {url?: string, content?: string}): VcfDataSource { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks for adding this implementation in. Can you remove the old one from the source code? |
||
var {url, content} = data; | ||
if (url) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tabs -> spaces |
||
return createFromVcfFile(new VcfFile(new RemoteFile(url))); | ||
} else if (content) { | ||
return createFromVcfFile(new VcfFile(new LocalStringFile(content))); | ||
} | ||
// If no URL or content is passed, fail | ||
throw new Error(`Missing URL or content from track: ${JSON.stringify(data)}`); | ||
} | ||
|
||
module.exports = { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
/* @flow */ | ||
'use strict'; | ||
|
||
import {expect} from 'chai'; | ||
|
||
import LocalStringFile from '../main/LocalStringFile'; | ||
import jBinary from 'jbinary'; | ||
|
||
describe('LocalStringFile', () => { | ||
function bufferToText(buf) { | ||
return new jBinary(buf).read('string'); | ||
} | ||
|
||
it('should fetch a subset of a file', function() { | ||
var f = new LocalStringFile('0123456789\n'); | ||
var promisedData = f.getBytes(4, 5); | ||
|
||
return promisedData.then(buf => { | ||
expect(buf.byteLength).to.equal(5); | ||
expect(bufferToText(buf)).to.equal('45678'); | ||
}); | ||
}); | ||
|
||
it('should fetch subsets from cache', function() { | ||
var f = new LocalStringFile('0123456789\n'); | ||
return f.getBytes(0, 10).then(buf => { | ||
expect(buf.byteLength).to.equal(10); | ||
expect(bufferToText(buf)).to.equal('0123456789'); | ||
return f.getBytes(4, 5).then(buf => { | ||
expect(buf.byteLength).to.equal(5); | ||
expect(bufferToText(buf)).to.equal('45678'); | ||
}); | ||
}); | ||
}); | ||
|
||
it('should fetch entire files', function() { | ||
var f = new LocalStringFile('0123456789\n'); | ||
return f.getAll().then(buf => { | ||
expect(buf.byteLength).to.equal(11); | ||
expect(bufferToText(buf)).to.equal('0123456789\n'); | ||
}); | ||
}); | ||
|
||
it('should determine file lengths', function() { | ||
var f = new LocalStringFile('0123456789\n'); | ||
return f.getSize().then(size => { | ||
expect(size).to.equal(11); | ||
}); | ||
}); | ||
|
||
it('should get file lengths from full requests', function() { | ||
var f = new LocalStringFile('0123456789\n'); | ||
return f.getAll().then(buf => { | ||
return f.getSize().then(size => { | ||
expect(size).to.equal(11); | ||
}); | ||
}); | ||
}); | ||
|
||
it('should get file lengths from range requests', function() { | ||
var f = new LocalStringFile('0123456789\n'); | ||
return f.getBytes(4, 5).then(buf => { | ||
return f.getSize().then(size => { | ||
expect(size).to.equal(11); | ||
}); | ||
}); | ||
}); | ||
|
||
it('should cache requests for full files', function() { | ||
var f = new LocalStringFile('0123456789\n'); | ||
f.getAll().then(buf => { | ||
expect(buf.byteLength).to.equal(11); | ||
expect(bufferToText(buf)).to.equal('0123456789\n'); | ||
return f.getAll().then(buf => { | ||
expect(buf.byteLength).to.equal(11); | ||
expect(bufferToText(buf)).to.equal('0123456789\n'); | ||
}); | ||
}); | ||
}); | ||
|
||
it('should serve range requests from cache after getAll', function() { | ||
var f = new LocalStringFile('0123456789\n'); | ||
return f.getAll().then(buf => { | ||
expect(buf.byteLength).to.equal(11); | ||
expect(bufferToText(buf)).to.equal('0123456789\n'); | ||
return f.getBytes(4, 5).then(buf => { | ||
expect(buf.byteLength).to.equal(5); | ||
expect(bufferToText(buf)).to.equal('45678'); | ||
}); | ||
}); | ||
}); | ||
|
||
it('should truncate requests past EOF', function() { | ||
var f = new LocalStringFile('0123456789\n'); | ||
var promisedData = f.getBytes(4, 100); | ||
|
||
return promisedData.then(buf => { | ||
expect(buf.byteLength).to.equal(7); | ||
expect(bufferToText(buf)).to.equal('456789\n'); | ||
return f.getBytes(6, 90).then(buf => { | ||
expect(buf.byteLength).to.equal(5); | ||
expect(bufferToText(buf)).to.equal('6789\n'); | ||
}); | ||
}); | ||
}); | ||
|
||
it('should fetch entire files as strings', function() { | ||
var f = new LocalStringFile('0123456789\n'); | ||
return f.getAllString().then(txt => { | ||
expect(txt).to.equal('0123456789\n'); | ||
}); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about using optional field annotations instead? Maybe something like this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure,
As I mentioned. I'm not JavaScript developer and Ecmascript 2015 is definitely new to me. I didn't know that I can do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries - I, also, had to check the
flow-type
reference and try a few things before I get that right. Not the most obvious solution ;)