appy-one / acebase-server

A fast, low memory, transactional, index & query enabled NoSQL database server for node.js that easily syncs with browser and node.js clients and servers
MIT License
32 stars 14 forks source link

Attach to an existing server. Route prefixing (#49) #55

Closed Azarattum closed 1 year ago

Azarattum commented 1 year ago

Some changes that were discussed in #49 were made here. I added a server option that accepts an http server and a route option which overrides the base route for AceBase.

For example (modified test.ts):

// Starts a test server for debugging sessions
import { createServer } from 'http';
import { AceBaseServer } from './server';

// Make sure development environment is set
process.env.NODE_ENV = 'development';

const server = createServer();
server.listen(8080);

const base = new AceBaseServer('default', { logLevel: 'verbose', route: '/acebase', host: 'localhost', port: 8080, server, path: '.' });
base.once('ready', () => {
    console.log(`Ready to debug!`);
});

process.on("SIGINT", () => server.close());

It will use server as it's server and listen on http://localhost:8080/acebase. Socket.IO will listen on http://localhost:8080/acebase/ws.

IMPORTANT! BREAKING CHANGE!

The default path for socket.io is now http://localhost:8080/ws! (Previously was http://localhost:8080/socket.io). In general the path now is http://localhost:8080/ + route + /ws.

AceBase didn't specify any path before for Socket.IO to use, therefore it fellback to /socket.io. We might want to change the new version to http://localhost:8080/ + route + /socket.io to preserve compatibility. But since this PR needs client changes anyway I thought using ws would be nicer. Also I've seen some comments in the code about supporting other web socket libraries. I that case /ws would be more future proof path than /socket.io.

I haven't started working on the client yet. Let me know you thoughts about this PR. What shall we do with socket.io? Are the other changes ok? I've also fixed the redirect to webmanager. Are there any other path dependent parts that I might have missed?

appy-one commented 1 year ago

Thanks for your PR! I'll take a look asap

appy-one commented 1 year ago

@Azarattum thanks for putting time into this. I do think it is not a good idea to introduce breaking changes, the websocket endpoint should remain at the same location when not using a route prefix to keep current clients working. Another reason to keep it named "socket.io" is because it's not compatible with other websocket libraries (/plain websockets) - switching to another websocket library requires clients to switch too. Keeping them at separate endpoints would allow supporting multiple websocket libraries simultaneously

I see you are assigning a router to env.app to automatically add the prefix to all REST endpoints, awesome! I do think that breaks your webmanager redirect though, does it?

Azarattum commented 1 year ago

@appy-one redirects are absolute paths. So, I did src/routes/webmanager.ts to fix the web manager.

appy-one commented 1 year ago

@Azarattum I mean the binding to env.app.get('/' .. where env.app already has the prefixed route, right?

Azarattum commented 1 year ago

@appy-one, yes, the webmanager's env.app.get('/' ... is prefixed. So, http://localhost/acebase redirects to http://localhost/acebase/webmanager when the prefix is acebase. I've added env.root + webManagerDir.slice(1) exactly for this purpose.

Azarattum commented 1 year ago

@appy-one, I fixed everything except for https://github.com/appy-one/acebase-server/pull/55#discussion_r1033397685, check the thread there

Azarattum commented 1 year ago

@appy-one, everything is resolved here.

appy-one commented 1 year ago

Thanks @Azarattum 👍🏼