Skip to content
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

Adds ability to override mount with publicServerURL for production uses #1287

Merged
merged 2 commits into from
Mar 31, 2016

Conversation

flovilmart
Copy link
Contributor

superseds #1270
fixes #1131

also adds a sanity so we don't keep trailing / in mount or publicServerURL

@flovilmart flovilmart force-pushed the flovilmart.dynamicConfigMount branch from 34051bd to ab18586 Compare March 31, 2016 00:09
@codecov-io
Copy link

Current coverage is 93.02%

Merging #1287 into master will not affect coverage as of a929dde

@@            master   #1287   diff @@
======================================
  Files           84      84       
  Stmts         5275    5289    +14
  Branches       960     965     +5
  Methods          0       0       
======================================
+ Hit           4907    4920    +13
  Partial         11      11       
- Missed         357     358     +1

Review entire Coverage Diff as of a929dde

Powered by Codecov. Updated on successful CI builds.

@drew-gross
Copy link
Contributor

I think we actually don't want to do this, since we don't really want internal queries to go back out to the public internet to do dns resolution and so on.

@flovilmart
Copy link
Contributor Author

We only use .mount to give back to the clients (and in file adapters)

@flovilmart
Copy link
Contributor Author

RestWrite.prototype.location = function() {
  var middle = (this.className === '_User' ? '/users/' :
                '/classes/' + this.className + '/');
  return this.config.mount + middle + this.data.objectId;
};

It's the only occurrence besides GridStoreAdapter and all FilesAdapter which both need public URL's otherwise create files that are referenced by the requested host i.e.: localhost:1337

And RestWrite.prototype.location is only used to fill the responses of object creation and OAuth login, never internally AFAIK.

Do you see other cases?

@drew-gross
Copy link
Contributor

Ah right this is only for the mount point, not the URL. In that case this looks good.

@drew-gross drew-gross merged commit 8a6903a into master Mar 31, 2016
@drew-gross drew-gross deleted the flovilmart.dynamicConfigMount branch March 31, 2016 03:29
@flovilmart
Copy link
Contributor Author

This is the full URL that is returned to the clients, mount is currently inferred :

function handleParseHeaders(req, res, next) {
  var mountPathLength = req.originalUrl.length - req.url.length;
  var mountPath = req.originalUrl.slice(0, mountPathLength);
  var mount = req.protocol + '://' + req.get('host') + mountPath;
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GridStoreAdapter files location
5 participants