outmoded / university

Community learning experiment
Other
371 stars 194 forks source link

Potential [Assignment 5] Glue and HTML views #132

Closed zoe-1 closed 9 years ago

zoe-1 commented 9 years ago

@hueniverse and community, below is a potential assignment to keep the project moving. @hueniverse Hopefully, you will find this acceptable. Your feedback would be appreciated. This should build off previous assignments and move us one step forward to master the learning objectives described at: https://github.com/zoe-1/hapitimes.

Community your feedback would be appreciated too.

[Assignment 5] Glue and HTML views

zoe-1 commented 9 years ago

If we want to simplify the assignment, we could split it into two. One assignment would be creating a plugin that renders html templates. The other assignment would be adding Glue support. But, I think everyone can accomplish the above objectives :-)

AdriVanHoudt commented 9 years ago

I can see the follow up with glue but I don't think rendering views is. Moving the registration of hapi-auth-basic out of private doesn't make sense, you're changing the architecture of the server just to use glue, you can just load private.js with glue and let it still register hapi-auth-basic. It's the responsibility of the private plugin to register it, no need to change that. Also I would really like to have an 'end goal' with the server so we know what to works towards instead of making "random" assignments. I think validation and or logging are more interesting at this point or to actually follow up on the previous assignment make use of a proper db and validation function for validation users Thanks anyway for all the effort @zoe-1!

zoe-1 commented 9 years ago

@AdriVanHoudt thanks for the input.

End Goal: Is to build an application that has the following features:

In the end the application will have the following routes:


Validation versus rendering views

In order to begin writing code that validates submitted data, we need to create HTML pages that submit form data (POST requests) or links (GET requests). Then, we can write validation code that tests the submitted data. Hence, we first need to make a plugin that renders or serves HTML views. Then, we can make code that validates submissions made from these HTML pages. Otherwise, where are we going to make POST and GET requests from? We could validate requests from curl. But, I think most people would be more interested in how to make a web server that serves HTML pages or renders HTML templates. And, they would want to know how to handle POST and GET requests made from those pages / HTML templates. For more about rendering HTML templates please read hapijs tutorial on views

Why move hapi-auth-basic registration logic

There are several reasons to move the registration logic out of the private plugin.

logging

Yes, we are going to add server logging eventually.

Making use of a proper DB.

Using a proper DB is super important. But, it should come after reaching the end goal described above. For now the plan is to use JSON objects as our data source. If we accomplish the end goal described above, users will have a clear example of how to make a hapijs web server with some core security features built in like: CSRF tokens and cookies encrypted using Iron. Using a proper DB right now would muddle our mastery of the hapijs framework. For now, let's just focus on hapi.

After we achieve our end goal

If we complete the above application, the hapijs community will have boilerplate code that many applications will be able to use regardless of the DB. We could then take that boilerplate code and start a new project like a todo application using MongoDB. It would feature user administration (CRUD) and todo list management.

@AdriVanHoudt and community. Please share your thoughts :-)

AdriVanHoudt commented 9 years ago

First of all, wow :D Second I like the proposal for the end goal, seems pretty easy (as to complexity of business logic) and allows us to make use of all the hapi features (for as far as I know them) Third, I do not agree with you on your reasoning behind separation of logic, my reasoning is also based on it but results in something different. Private.js is not just for route handling, it deals with all the private routes. This means it needs some form of authentication, in this case hapi-auth-basic. Other routes (like login in the future) won't need hapi-auth-basic. Leaving the responsibility of authentication to the plugin that needs it seems reasonable to me. you could go as far as saying that the plugin needs to be, well, plugable. Meaning that when you would for example use the plugin in another server it has everything it needs to work. Now of course there are cases where this is not true and plugins rely on other plugins and maybe multiple plugins need the hapi-auth-basic or something but I feel that in this case it teaches separation of concern. Also the moment you feel it's getting buried there is something wrong. The registration sits in the register function of the plugin so it's not like it's hidden away. Also the code we have now doesn't seem so ugly to me (https://github.com/hueniverse/hueniversity/blob/master/lib/private.js#L33) :P Fourth, I agree on the db stuff, it's not a part of hapi but a lot of people will want an example of it since every server practically needs it.

Last random thought, what if eventually this project will become the server that hosts the website that has the tutorial for this project?

FennNaten commented 9 years ago

@zoe-1 Nice work, looks like a very good roadmap for a start!

TLS, definitely must have.

I like the idea of composing the app with glue, I'm currently using it on a project at work and I'm sold on configuration-centric apps anyway. And it has some small gotchas, so it would be nice to provide in this project an example of how to handle them. This way of bootstrapping an app may not be appreciated by everyone though.

About the position of auth plugins registration, it's kind of a tricky subject as in real-world apps it would IMHO really depends on what kind of project you're working on and how your team is structured. If your working with a small team on a one-shot website, you can imagine having people responsible for different parts of the app materialized as plugins, and let each plugin handle its auth. But you can also be working with plugins used e.g. as reusable api endpoints for different projects that may need different auth depending on the app they're mounted on, and there your architecture would be different, with e.g. plugins having their auth given by the root at registration. I don't think there can be a silver bullet here, but it would be interesting to get @hueniverse input about the most common way to handle that with large projects running Hapi. (please, if you come by and have a few minutes ;) )

Some things I'd add to the areas of study:

That's all that comes to my mind at the moment.

AdriVanHoudt commented 9 years ago

Indeed there is no silver bullet but in our case I tend to lean towards keeping it in the plugin, for my daytime job I work alone on a server so in that case it doesn't really matter.

zoe-1 commented 9 years ago

@AdriVanHoudt and @FennNaten Thank you for sharing your feedback! It feels like we are beginning to get a good sense of where our project needs to go. :-)

In respect to building a tutorial website, I like the idea and hope we will do it. I wonder what @hueniverse thinks about it?

In respect to glue and the position of auth plugin(s) registration, there are two existing projects that provide example code of where to position the auth plugin(s) registration:

Both sites referenced above follow the style of registering and configuring authentication plugins outside of route handler plugins. And, this is style I recommend our project follow.

In respect to the other areas of study you both wrote about, I think this discussion is important and this kind of feedback will shape the future of our project. From my perspective most everything you both mentioned would be useful to build. Thanks for sharing your thoughts.

In respect integrating all those various topics you both metioned into one tutorial project, I am not quite sure what that would look like. If we accomplish our current goals and begin another phase of development I am sure we will have some good discussion about what to build next :-)

AdriVanHoudt commented 9 years ago

Well those 2 projects need the auth in multiple plugins, so then it makes sense to take it out, in our case the auth is only needed in the routes of private.

And yeah we need to come up with something that makes sense in the end and incorporates all the ideas ^^

idanwe commented 9 years ago

@zoe-1 Good lack with taking that project forward, I'm here to help too.

@AdriVanHoudt I also think that currently the hapi-auth-basic should be registered inside Private due that it is only used there.

About the next assignments, I would like to see glue plugin integration in a more advanced assignment. I think there is more fundamental issues that we didn't cover yet. In my opinion the next assignment should be complete cover of scheme and DB with tests. (that will also give a chance to showcases the server.bind function.) Every developer will want that, especially for new ones this "integration" is problematic, and it is missing good examples. In compare to express examples.

After that we will have a good base to add cache and documentation, as @FennNaten mentioned.

zoe-1 commented 9 years ago

@idanwe Good thoughts above :-) Glad you want to help. Your thoughts about the assignments would be good to do and are needed but most of those issues will be addressed in the next phase. The current development goal described above is very hapijs centric and illustrates basic components needed to build an application with hapi regardless of database. After we achieve that goal it would be a logical next step to do something database related. We will have built boilerplate code that many applications can use regardless of the DB. Then, we can begin to build an application / tutorial that illustrates hapi working with a database. Can you understand this point of view?

In respect, to your thoughts about the private module and hapi-auth-basic. The main reason for moving it out of the private plugin into it's own auth file is because we are planning to build skeleton code for other developers to use. We expect them to want to build complex applications needing authentication strategies. So, it would make sense to design the auth registration code so others will not have to change too much when they start adding 5-10 plugins into their project. :-)

My reasoning for using glue is it let's you easily add and manage plugins to your server. So, by using glue, we are designing with the expectation that future users (including us) will use this code to kickstart their hapi applications and want to add many plugins into the project.

glue is a must for a hapijs project with many plugins (IMHO). If you are not using glue you will end up writing your own code to do what glue does. glue exists because the hapijs community developed a solution to a common issue all hapijs projects face (composing your server with all plugins and configurations in place). It is a good solution :-)

Plus, I think it would be easier for people new to glue to add it to the project now before it gets complicated.

AdriVanHoudt commented 9 years ago

@idanwe has a fair point, I have a server with 10 plugins atm (should be more but I didn't have a tutorial to learn from ;) ) and it's fine without glue. Glue would indeed work better for more complex configurations. And good point that db can also incorporate schema validation!

@zoe-1 I do not agree with your 'main reason' of this project. My thought was that this was a way of learning Hapi. I see no reason to provide boilerplate code in a tutorial. Of course the tutorial will have code that can easily be 'copy & pasted' to use but that should not be the goal. The goal is to learn Hapi and all of it features and possibilities and in the process the 'student' (since we are a university now :D) will learn how to use Hapi for his own use cases.

AdriVanHoudt commented 9 years ago

@zoe-1 you are a diplomatic man ^^ I like how you suggested people could do the moving of the plugin as an extra exercise A few thoughts on the assignment:

hussainanjar commented 9 years ago

Hi @zoe-1 good job with new assignment and I like the roadmap, will be submitting a PR soon :+1:

zoe-1 commented 9 years ago

@hussainanjar Glad you like the assignment. Look forward to seeing your PR :-)

zoe-1 commented 9 years ago

@AdriVanHoudt and community, in response to @AdriVanHoudt questions and recommendations, I edited the assignment to clarify things and answer the questions. Thank you @AdriVanHoudt for the feedback! It is always good to have another set of eyes look things over.

In response to, no indication of what the home plugin should give back:
In response to the glue testing question, can we have 100% cc with that?

Yes, we can get 100% coverage. glue and lab play well together. But, we will have to refactor our tests to make it happen.

In response to making a label about discussion for next assignments

Yes, good idea. Plan to do it.

In response to adding a check part to the assignment.

That is a good idea :-) I will change it soon.

Why I chose not refactor the auth plugin registration myself after the assignment:
AdriVanHoudt commented 9 years ago

Thanks for all the work @zoe-1 I think I wasn't to clear myself about the refactoring, I was talking about removing the makefile etc not about the auth plugin

FennNaten commented 9 years ago

@zoe-1 Nice work with the assignment. I mostly agree with your points however I'd have two remarks:

hussainanjar commented 9 years ago

@AdriVanHoudt I think its better if I get to do the cleanup rather then someone doing on my behalf. It helps you learn best practices

AdriVanHoudt commented 9 years ago

Can't you pass an array to glue? And ok I get behind that, although when we convert it to a website with tutorials we need to shift those things :P

On Sun, 24 May 2015 06:52 Hussain Fakhruddin notifications@github.com wrote:

@AdriVanHoudt https://github.com/AdriVanHoudt I think its better if I get to do the cleanup rather then someone doing on my behalf. It helps you learn best practices

— Reply to this email directly or view it on GitHub https://github.com/hapijs/university/issues/132#issuecomment-104979345.

FennNaten commented 9 years ago

@AdriVanHoudt Nope, in its current state Glue won't take an array for plugins, it iterates over them using directly Object.keys on the property and using each key to resolve the plugin.

zoe-1 commented 9 years ago

@AdriVanHoudt In respect to the make file, I put the refactor in the assignment so everyone would see that hapijs style requirements were modified for the whole hapijs community.

@FennNaten in respect to your question about Glue and the order of plugins being registered, based upon my experience and testing, Glue registers the plugins in the order you place them in the manifest file. If you put the auth plugin registration configs at the bottom of Glue's plugin registry, this results in registration failure. However, if you place auth plugins first in the plugins section of manifest.js like the (aqua project)[https://github.com/jedireza/aqua/blob/master/manifest.js] everything works fine.

Later Comment Addition Below:

Sorry, I answered this question too fast. Just saw you started an issue at aqua repo about this. https://github.com/jedireza/aqua/issues/36 In short, what I described above is what worked for me. I am interested in what @hueniverse would say about this.

zoe-1 commented 9 years ago

Assignment Due May 30th.

@FennNaten thank you for the due date reminder :-)

FennNaten commented 9 years ago

@zoe-1 : about the order of plugins registration, it's more of a remark than a question ;) Replication of the issue is hard to do, I won't say it's random because it's not really, but it depends on how V8 engine 'decides' to optimize the code being executed. Most of the time, object keys are kept in the order they're inserted, and everything is fine. However, this is not a fact to take for granted, as ECMAScript spec doesn't guarantee insertion order preservation for object keys. It's not really important matter, just something to have in mind for bullet-proofing the code.

zoe-1 commented 9 years ago

@FennNaten It seems @marsup refers to the solution to the plugin load order at: https://github.com/hapijs/glue/issues/19 (you referenced this issue above). Looks like we need a JSON schema requiring dependecies to be loaded before plugins that depend on them are loaded. Or, we need to declare dependencies the glue way. But, I have never seen a real world example of glue doing this and the glue docs don't make any mention to it. But, @Marsup implies it is possible and that would seem to be the way to bullet proof the code.

FennNaten commented 9 years ago

@zoe-1 Yes, that's what I was saying in my original remark ;)

The consequence is that you must ensure plugin load order for the auth part, either by using dependency definition in plugin attributes or by manually registering on prePlugins Glue hook.

However I still have to make tests to see what works and what doesn't. Was planning to do them while working on optional part on the assignment

zoe-1 commented 9 years ago

@FennNaten Sometimes I have to read things a couple times before I actually read them LOL ;)

zoe-1 commented 9 years ago

@FennNaten Got feedback from @Marsup on gitter. He said to use: http://hapijs.com/api#serverdependencydependencies-after logic on the plugins that have dependencies. This will ensure all plugins are loaded with needed dependencies when composed by glue and avoids the order of plugin registration issue we have been talking about. This should bullet proof the registration code :-)

FennNaten commented 9 years ago

@zoe-1 I just added a home plugin and brought back 100% test coverage, I'll do some tests with dependencies while completing optional assignement, to be sure that works as expected.

Speaking of tests, in the assignment you say

Ensure 100% test coverage. Note, testing if a plugin loads is not practical with Glue. If a plugin described in the manifest file does not load, glue will throw errors and shut down the application nothing else will get executed. So, you cannot make tests like we did in earlier assignments to see if a plugin loads or not when using glue.

Glue uses CPS and actually bubbles up registration errors, if we follow the same pattern we've been using (with index.js exposing an init method taking the manifest and options as params + a callback with signature function(err, server)), then tests keep working in the exact same way ;)

zoe-1 commented 9 years ago

@FennNaten In respect to your observations about Glue using CPS (Continual Passing Style), and registration errors, it would be great to have our project test the success or failure of plugins loading just as we did before. Thank you for sharing your observations and knowledge. You are making the quality of our assignment improve! I look forward to seeing your PR.

FennNaten commented 9 years ago

@zoe-1 You're welcome, I just try to do my part ;)

AdriVanHoudt commented 9 years ago

@zoe-1 maybe start a thread about the next assignment? Also not much activity? Maybe promote this by putting it on twitter and the website or something?

On Thu, 28 May 2015 22:24 Jon Swenson notifications@github.com wrote:

Closed #132 https://github.com/hapijs/university/issues/132.

— Reply to this email directly or view it on GitHub https://github.com/hapijs/university/issues/132#event-316581281.

zoe-1 commented 9 years ago

@AdriVanHoudt good recommendations. In respect, to promoting on twitter etc. I believe @hueniverse manages all that.

hueniverse commented 9 years ago

A few people have access to the twitter account but if you just promote it on twitter and mention @hapijs I will retweet it. @arb and @geek also have access (and I'm happy to add more people if someone wants to help maintain it and keep the updates going without overdoing it).

AdriVanHoudt commented 9 years ago

OK cool and I'll see where it can fit on the website

On Fri, 29 May 2015 02:14 Eran Hammer notifications@github.com wrote:

A few people have access to the twitter account but if you just promote it on twitter and mention @hapijs https://github.com/hapijs I will retweet it. @arb https://github.com/arb and @geek https://github.com/geek also have access (and I'm happy to add more people if someone wants to help maintain it and keep the updates going without overdoing it).

— Reply to this email directly or view it on GitHub https://github.com/hapijs/university/issues/132#issuecomment-106638810.