Skip to content

CJ Browser update #4

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 9, 2014
Merged

CJ Browser update #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
502b8eb
Add Delete support.
jvelilla Feb 4, 2014
e288d17
Set up a bigger timeout.
jvelilla Apr 9, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/cj_client_proxy.e
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,10 @@ feature -- Execution
Result := client.update_with_template (a_path, tpl, adapted_context (ctx))
end


delete (a_path: READABLE_STRING_GENERAL; ctx: detachable HTTP_CLIENT_REQUEST_CONTEXT): CJ_CLIENT_RESPONSE
do
Result := client.delete (a_path, adapted_context (ctx))
end

end
43 changes: 40 additions & 3 deletions src/cj_template_tool.e
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,12 @@ feature -- Change
v: EV_VERTICAL_BOX
hb: EV_HORIZONTAL_BOX
lab: EV_LABEL
tf: EV_TEXT_FIELD
-- tf: EV_TEXT_FIELD
tf: EV_TEXT
tf_passwd: EV_PASSWORD_FIELD
but: EV_BUTTON
table: HASH_TABLE [EV_TEXT_FIELD, STRING_32]
but_delete: detachable EV_BUTTON
table: HASH_TABLE [EV_TEXT_COMPONENT, STRING_32]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should use STRING_TABLE (not critical)

is_creation: BOOLEAN
is_password: BOOLEAN
do
Expand Down Expand Up @@ -110,15 +112,23 @@ feature -- Change
else
create but.make_with_text ("Update")
but.select_actions.extend (agent on_post (coll, tpl, table, False))
create but_delete.make_with_text ("Delete")
but_delete.select_actions.extend (agent on_delete (coll, tpl, table))

end

v.extend (create {EV_CELL})
v.extend (but)
if but_delete /= Void then
v.extend (but_delete)
v.disable_item_expand (but_delete)

end
v.disable_item_expand (but)
-- FIXME
end

on_post (coll: CJ_COLLECTION; tpl: CJ_TEMPLATE; table: HASH_TABLE [EV_TEXT_FIELD, STRING_32]; is_creation: BOOLEAN)
on_post (coll: CJ_COLLECTION; tpl: CJ_TEMPLATE; table: HASH_TABLE [EV_TEXT_COMPONENT, STRING_32]; is_creation: BOOLEAN)
local
ctx: HTTP_CLIENT_REQUEST_CONTEXT
l_href: STRING_8
Expand Down Expand Up @@ -147,6 +157,33 @@ feature -- Change
-- dlg.focus_out_actions.extend (agent dlg.destroy_and_exit_if_last)
end

on_delete (coll: CJ_COLLECTION; tpl: CJ_TEMPLATE; table: HASH_TABLE [EV_TEXT_COMPONENT, STRING_32])
local
ctx: HTTP_CLIENT_REQUEST_CONTEXT
l_href: STRING_8
dlg: EV_INFORMATION_DIALOG
resp: CJ_CLIENT_RESPONSE
do
create ctx.make
across
tpl.data as c
loop
if attached table.item (c.item.name) as tf then
c.item.set_value (tf.text)
end
end
if attached coll.items as l_items and then attached l_items.first as first_item then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a comment, usually when we call .first ... we should first check that the list is not empty, otherwise the precondition is violated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is now fixed in the new branch, not yet pushed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, you can push right away, no need to go via pull request for this one.

On Wed, Apr 9, 2014 at 4:12 PM, jvelilla notifications@github.com wrote:

In src/cj_template_tool.e:

  • on_delete (coll: CJ_COLLECTION; tpl: CJ_TEMPLATE; table: HASH_TABLE [EV_TEXT_COMPONENT, STRING_32])
  •   local

  •       ctx: HTTP_CLIENT_REQUEST_CONTEXT

  •       l_href: STRING_8

  •       dlg: EV_INFORMATION_DIALOG

  •       resp: CJ_CLIENT_RESPONSE

  •   do

  •       create ctx.make

  •       across

  •           tpl.data as c

  •       loop

  •           if attached table.item (c.item.name) as tf then

  •               c.item.set_value (tf.text)

  •           end

  •       end

  •       if attached coll.items as l_items and then attached l_items.first as first_item then

The code is now fixed in the new branch, not yet pushed.

Reply to this email directly or view it on GitHubhttps://github.com//pull/4/files#r11437080
.

l_href := first_item.href
resp := cj_client.delete(l_href, Void )
else
l_href := coll.href
resp := cj_client.delete (l_href, Void)
end
create dlg.make_with_text ("Result")
dlg.set_text (resp.http_response)
dlg.show
end

invariant
widget_attached: widget /= Void

Expand Down
41 changes: 41 additions & 0 deletions src/kernel/cj_client.e
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ feature {NONE} -- Initialization
-- Initialize `Current'.
do
create {LIBCURL_HTTP_CLIENT_SESSION} client.make (a_service) --"http://jfiat.dyndns.org:8190")
client.set_timeout (25)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should have a preference/config file for that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree

end

feature -- Access
Expand Down Expand Up @@ -71,6 +72,7 @@ feature -- Access
if l_ctx = Void then
create l_ctx.make
end
l_ctx.add_header ("Accept", "application/vnd.collection+json")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it make sense to specialize the "vnd.collection+json.my_version" for a specific api
if yes, this header could also be a preference, or a field in the UI

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe,but I'm not sure.

l_ctx.add_header ("Content-Type", "application/vnd.collection+json")

if attached cj_template_to_json (tpl) as j then
Expand Down Expand Up @@ -112,6 +114,8 @@ feature -- Access
create l_ctx.make
end
l_ctx.add_header ("Content-Type", "application/vnd.collection+json")
l_ctx.add_header ("Accept", "application/vnd.collection+json")


if attached cj_template_to_json (tpl) as j then
d := "{ %"template%": " + j.representation + " }"
Expand Down Expand Up @@ -151,6 +155,8 @@ feature -- Access
create l_ctx.make
end
l_ctx.add_header ("Content-Type", "application/vnd.collection+json")
l_ctx.add_header ("Accept", "application/vnd.collection+json")


if attached q.data as q_data then
across
Expand Down Expand Up @@ -185,6 +191,41 @@ feature -- Access
end


delete (a_path: READABLE_STRING_GENERAL; ctx: detachable HTTP_CLIENT_REQUEST_CONTEXT): CJ_CLIENT_RESPONSE
local
l_http_response: STRING_8
j_body: like json
l_formatted_body: detachable STRING_8
col: detachable CJ_COLLECTION
l_ctx: detachable HTTP_CLIENT_REQUEST_CONTEXT
l_url: STRING_8
do
create l_http_response.make_empty
l_ctx := ctx
if l_ctx = Void then
create l_ctx.make
end
l_ctx.add_header ("Accept", "application/vnd.collection+json")
l_url := a_path.to_string_8
if attached client.delete (l_url, l_ctx) as g_response then
l_url := g_response.url
l_http_response.append ("Status: " + g_response.status.out + "%N")
l_http_response.append (g_response.raw_header)
if attached g_response.body as l_body then
l_http_response.append ("%N%N")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we also building the response manually for the other?
if yes, maybe we should have a CLIENT_RESPONSE_MESSAGE to check the message is valid, and to handle the %N%N ... (which should be %R%N%R%N by the way) ...

and so on

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we are doing it by hand, so I think it's a good idea to move this responsability into a new class

l_http_response.append (l_body)
if attached json (l_body) as j then
j_body := j
col := cj_collection (j)
end
else
l_formatted_body := Void
end
end
create Result.make (l_url, l_http_response, j_body, col)
end


feature {NONE} -- Implementation

shared_ejson: SHARED_EJSON
Expand Down