riboseinc / digicert

Ruby bindings for the Digicert Services API
https://www.digicert.com/services/v2/documentation
MIT License
8 stars 10 forks source link

Add `#fetch_to_path` for order instance #65

Closed abunashir closed 7 years ago

abunashir commented 7 years ago

This commit adds the #fetch_to_path for order instance, so if you do not have the certificate_id but you only have the order id then you can use order instance to download the certificate to a specific path.

order = Digicert::Order.find(order_id)
order.fetch_to_path(path: File.expand_path("tmp"), ext: "zip")

Ref #61

abunashir commented 7 years ago

I think I was missing something, just noticed, duplicating an order returns the same existing order_id, I thought they will return a new order id but it's not, so the certificate_id here is not right, I will push more changes on this soon.

ronaldtse commented 7 years ago

Right, maybe use the request's results' date_created to correlate with the listing of duplicated certificate?

abunashir commented 7 years ago

Yap, but what if someone has two certificate duplication request at the same day?

What about we list all the duplication and find the certificate with the highest id, for example, latest certificate, is most likely gonna have the incremented id then the other one. Maybe something like

certificates = Digicert::DuplicateCertificate.all(order_id: order_id)

# select the maximum certificate id
certificates.map(&:id).max
# => [1, 2] -> 2 :)
ronaldtse commented 7 years ago

According to the Digicert API, date_created is the time of creation so rare to have both identical:

"date_created": "2014-08-19T18:16:07+00:00",

If we just find the latest ID it won't work if we want to generating 2 new certificates at once and download them together. Of course we hope to parallelize the creation and download of duplicate certificates if possible.

abunashir commented 7 years ago

That's true.

At a time we are downloading one certificate, right?

ronaldtse commented 7 years ago

It is possible that there are two processes that are duplicating certificates at the same time, so we should track some unique identifier of the request to certificate (just unfortunate they don't provide certificate ID upon the request).

abunashir commented 7 years ago

Yes, It's kind of made it complex on our end.

In the view request section, there are two date_created, once is under order node and another one is under certificate node, which one should we use?

Link: https://www.digicert.com/services/v2/documentation/request/request

ronaldtse commented 7 years ago

I think the steps are:

view_request_response = my_order.request_duplicate
newly_created_time = view_request_response.certificate.date_created
duplicated_certificate = list_duplicate_certificates.find(newly_created_time)

So probably the request.certificate.date_created .

abunashir commented 7 years ago

cool, I will add this later today!

abunashir commented 7 years ago

Hi @ronaldtse , I have been working on this interface for a while and now user can download the duplicate certificate like this but I realised couple of interesting things (please see the note bellow)

# Duplicate an order
order = Digicert::Order.find(order_id)
duplicate_order = order.duplicate

# Find the duplicate certificate
certificate = Digicert::DuplicateCertificateFinder.find_by(
  request_id: duplicate_order.requests.first.id,
)

# Download the certificate to a path
order.fetch_to_path(certificate.id, path: "file/path", ext: "zip")

I feel like this behavior does not belong to the order instance, as certificate downloader requires a certificate_id and it has nothing to do with the order_id. The certificate_id retrieval process is dependent in order, but not the downloading. When we retrieve an order Order::fetch(order_id) then the response object includes certificate.id

I think maybe we should create a Digicert::Certificate class and add the certificate instance methods there, like download, fetch_to_path, what do you think?

ronaldtse commented 7 years ago

Good idea Abu, let's use the Certificate class for these methods.

abunashir commented 7 years ago

Added in PR #68 and PR #69.