Second review of gpii.express module... #2

the-t-in-rtf · 2015-03-10T15:20:27Z

I have completed another round of updates since our last discussion, and am submitting my work for review.

The biggest change is that gpii.express is now responsible for wiring all middleware and components together.

…last conversation with Antranig. Attempted to migrate to using distributeOptions instead of component inspection. This works fine for middleware components (which are not nested), but does not work for router components (which can be nested). Will have to keep the previous approach until I either get advice for the group or can talk with Antranig again.

Conflicts: src/js/json.js src/js/urlencoded.js

…dependency from test "reqView" class. Added configuration and tests to test middleware being inappropriately exposed from either a child or a sibling.

…ving middleware isolation, per feedback from Antranig on the mailing list.

…d other common test components.

… for same.

…arams with GET calls, for example).

… This is required to support GET /path/:param style bindings.

…f all-tests.js to use the IoC test framework.

…ranig.

amb26 · 2015-03-11T19:04:51Z

README.md

@@ -1 +1,2 @@
-placeholder, please ignore.
+# gpii-express
+A fluid module to load express.js


This should be fleshed out a little by explaining the particular goals and strategy of the integration. For example - our desire to have containment structure of Infusion component trees to mirror the mounting structure of express routers and middleware, the ability to access Infusion's facilities for inspecting and advising compound structures by integrators who want to bundle and modify existing apps, etc. Should also contrast this with Kettle, and emphasise that, for now, this is a less ambitious and more concrete integration that doesn't give access to any fresh request-scoped material (IoC components) beyond those which express already supplies - that is, its native request and response objects.

Sorry, that was boilerplate that I obviously missed updating. This has now been greatly expanded.

…l request.

…ner definitions per pull request feedback.

…ization code.

…per pull request feedback.

amb26 · 2015-03-25T19:49:21Z

src/js/express.js

+                    component.options.router.use(component.options.path, childComponent.getMiddleware());
+                }
+                else {
+                    fluid.fail(new Error({message: "A component must expose a router in order to work with child middleware components."}));


Simply fluid.fail("A component etc." .... is fine

…sewhere.

…ixed whitespace.

…ntered.

…sewhere.

…hat actually needs it.

the-t-in-rtf · 2015-03-31T10:17:21Z

@amb26, we are nearly done, I think. I could use feedback on the testCaseHolder, I couldn't wire in the two common steps to each sequence without encountering errors. The non-working configuration is there, but commented out.

the-t-in-rtf · 2015-04-03T08:51:08Z

Antranig's comments via Skype:

[3/31/15, 3:43:34 PM] Antranig Basman: Yes, you'll get horribly confused trying to nest your test defs inside an expander
[3/31/15, 3:43:51 PM] Antranig Basman: Instead you should use the "moduleSource" option where you get to write an arbitrary function returning your test cases rather than "module"
[3/31/15, 3:44:32 PM] Antranig Basman: Well
[3/31/15, 3:44:43 PM] Antranig Basman: Either that or just write a function which returns the config wholesale
[3/31/15, 3:44:58 PM] Antranig Basman: https://github.com/fluid-project/kettle/blob/master/lib/test/KettleTestUtils.js#L472
[3/31/15, 3:45:00 PM] Antranig Basman: Is a good example

…quences.

the-t-in-rtf · 2015-04-07T10:04:30Z

The testCaseHolder has now been updated to wire in the common setup test sequence steps automatically.

amb26 · 2015-04-07T20:39:45Z

Congratulations, ADTKINS on having reached this great milestone on such a long and tortuous road :)

the-t-in-rtf added 16 commits February 20, 2015 14:28

Merge branch 'master' of https://github.com/the-t-in-rtf/gpii-express

787726e

Conflicts: src/js/json.js src/js/urlencoded.js

GPII-1099: Working to test middleware isolation. Removed req.session …

1eb5a28

…dependency from test "reqView" class. Added configuration and tests to test middleware being inappropriately exposed from either a child or a sibling.

GPII-1099: Refactored to enable clean nesting of routers while preser…

af256b2

…ving middleware isolation, per feedback from Antranig on the mailing list.

GPII-1099: Attempted to rewrite tests using a fluid testCaseHolder an…

2c313ef

…d other common test components.

Improved handling when gpii.express is itself nested, and added tests…

2ad3810

… for same.

Added model relay to the gradeNames.

7ede267

Added support for method-specific routers (you can only support req.p…

32bd6d5

…arams with GET calls, for example).

Added tests for parameter handling and options to control the method.…

eccf996

… This is required to support GET /path/:param style bindings.

Added index.js files per feedback from Antranig.

75217b2

Added standard grunt configuration per feedback from Antranig.

c233d60

Cleaned up all errors reported by JSHint. Also completed conversion o…

7ce82ca

…f all-tests.js to use the IoC test framework.

Completed test refactor for gpii.express module per feedback from Ant…

6916da0

…ranig.

Updated grunt-gpii dependency to fix bug with dedupe-infusion.

ec72860

Added safety check for poorly-wired nested modules.

c2b4919

Moved kettle to dev dependencies.

60aa344

the-t-in-rtf assigned amb26 Mar 10, 2015

amb26 reviewed Mar 11, 2015
View reviewed changes

the-t-in-rtf added 6 commits March 18, 2015 11:11

Removed previous model cruft and simplified the gradeNames as well.

d0e9812

Renamed variables in connectDirectDescendants per feedback from pul…

b519858

…l request.

Namespaced onDestroy handler and removed unneeded arrays from liste…

8c47e6d

…ner definitions per pull request feedback.

Added local port variable to improve readability of express initial…

436b2cc

…ization code.

Consolidated all "content checking" functions into a single function …

e4fe4c9

…per pull request feedback.

Committed work in progress on refactor to get feedback from Antranig.

419393c

amb26 reviewed Mar 25, 2015
View reviewed changes

duhrer added 6 commits March 31, 2015 12:13

Moved isSaneResponse to its own helper module that can be reused el…

eb1c7ab

…sewhere.

Updated dependencies (while troubleshooting unrelated problems) and f…

f5cc8f7

…ixed whitespace.

Migrated from using fluid.log to fluid.fail when errors are encou…

6743708

…ntered.

Fixed whitespacing.

0afec7d

Moved isSaneResponse to its own helper module that can be reused el…

c579105

…sewhere.

Moved fluid.modelRelayComponent grade to just the test middleware t…

dbb1f38

…hat actually needs it.

Updated testCaseHolder to wire in common "setup" steps to all test se…

d2b57d7

…quences.

amb26 merged commit d2b57d7 into fluid-project:master Apr 7, 2015

the-t-in-rtf deleted the second-review branch September 2, 2015 11:05

the-t-in-rtf restored the second-review branch February 8, 2016 10:02

the-t-in-rtf deleted the second-review branch April 21, 2017 14:32

the-t-in-rtf restored the second-review branch June 5, 2018 11:37

the-t-in-rtf deleted the second-review branch June 5, 2018 11:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Second review of gpii.express module... #2

Second review of gpii.express module... #2

the-t-in-rtf commented Mar 10, 2015

amb26 Mar 11, 2015

the-t-in-rtf Mar 17, 2015

amb26 Mar 25, 2015

the-t-in-rtf Mar 31, 2015

the-t-in-rtf commented Mar 31, 2015

the-t-in-rtf commented Apr 3, 2015

the-t-in-rtf commented Apr 7, 2015

amb26 commented Apr 7, 2015

Second review of gpii.express module... #2

Second review of gpii.express module... #2

Conversation

the-t-in-rtf commented Mar 10, 2015

amb26 Mar 11, 2015

Choose a reason for hiding this comment

the-t-in-rtf Mar 17, 2015

Choose a reason for hiding this comment

amb26 Mar 25, 2015

Choose a reason for hiding this comment

the-t-in-rtf Mar 31, 2015

Choose a reason for hiding this comment

the-t-in-rtf commented Mar 31, 2015

the-t-in-rtf commented Apr 3, 2015

the-t-in-rtf commented Apr 7, 2015

amb26 commented Apr 7, 2015