Ayende @ Rahien

It's a girl

Interview question: That annoying 3rd party service

We are still in a hiring mode. And today we have completed a new question for the candidates. The task itself is pretty simple, create a form for logging in or creating a new user.

Seems simple enough, I think. But there is a small catch. We’ll provide you the “backend” for this task, which you have to work with. The interface looks like this:

public interface IUserService
{
        void CreateNewUser(User u);
 
        User GetUser(string userId);
}

public class User
{
   public string Name {get;set;}
   public string Email {get;set;}
   public byte[] Sha1HashedPassword {get;set;}
}

The catch here is that we provide that as a dll that include the implementation for this, and as this is supposed to represent a 3rd party service, we made it behave like that. Sometimes the service will take a long time to run. Sometimes it will throw an error (ThisIsTuesdayException), sometime it will take a long time to run and throw an error, etc.

Now, the question is, what is it that I’m looking to learn from the candidate’s code?

Comments

Joey
08/27/2014 09:20 AM by
Joey

Exception handling?

Boris
08/27/2014 09:26 AM by
Boris

Dependency Injection?

Max
08/27/2014 09:27 AM by
Max

Defensive? Stable user experience?

Boris
08/27/2014 09:27 AM by
Boris

MVC pattern

eclaus
08/27/2014 09:34 AM by
eclaus

What level of trust does he give to the world outside ?

Martin Doms
08/27/2014 09:44 AM by
Martin Doms
  • Handling of long-running processes - probably using async/await for the .NET code, maybe perform the logic asynchronously from the client (Ajax call).
  • Handling of exceptions - can actually be a little bit tricky if the candidate uses async/await (just a matter of knowing where the exception occurs - on the await, not on the method call).
  • Sanitising user input. Do you trust the third party library to sanitise the user's name? The candidate will need to ask some smart questions - where does HTML encoding/decoding happen? Do I need to handle it in my code, or will I end up double-encoding? This applies to both methods.
  • Validation of the email address. Does the library handle this? If so, how? Throw an exception presumably, but does the exception contain information on which field failed validation and why? If the library doesn't handle validation then we need to. Probably some validation on user name too.

Basically it's the same issues you need to tackle on any CRUD but I think you're excepting smart questions more than smart implementations.

Luke
08/27/2014 10:00 AM by
Luke

Maybe how he handles exceptions?

Dave
08/27/2014 10:10 AM by
Dave

Well, let's first start with that the IUserService itself is incomplete. There is no way to know the userID as the User entity does not contains this field and CreateUser returns a void. This makes it impossible to ever perform a GetUser call.

I guess you want to see how the candidates code handles unreliable possibly blocking calls to an (unknown) service. The exceptions are just a trap. It's never about the exceptions itself. Everything can (and will) fail.

I think you want to see how much 'wrapper' code the candidate writes around the pesky service to deal with common network related exceptions.

matt
08/27/2014 10:30 AM by
matt

That they tell you friends don't let friends use sha1 for password storage?

Jef Claes
08/27/2014 10:42 AM by
Jef Claes

Does he know how to use a disassembler?

Rik Hemsley
08/27/2014 11:12 AM by
Rik Hemsley

Nice test. When I write tests for developers, they're always based on real scenarios I've had to deal with, and I always have a go at them myself.

Does this test pass those tests? As Dave mentions, it doesn't seem possible to get a user ID to pass to GetUser, though you may have been recreating this from memory rather than copying and pasting, of course.

Rob
08/27/2014 11:19 AM by
Rob

A nice anti-corruption layer? I'd probably be looking for something that did a couple of retries (for transient errors), had a single place where userId was assumed and good error handling.

By "a single place where userId was assumed" I mean that you have two options, either the name or the email address must be the user id. In the wrapper I'd want to make the userId explicit by having a method like:

private string GetUserId(User u) { return u.Email; }

Ayende Rahien
08/27/2014 12:01 PM by
Ayende Rahien

Dave, the user id (which we give the candidate as part of the test) is the email address

Gilligan
08/27/2014 12:32 PM by
Gilligan

Caching and retry logic, I'd imagine.

Rémi Bourgarel
08/27/2014 12:43 PM by
Rémi Bourgarel
  • Handling of the results / exception in an async situation
  • If he tries to instantiate the service himself
  • how he creates the content of "Sha1HashedPassword " from the user input ?
Paul
08/27/2014 01:26 PM by
Paul

Martin Doms answer and also effectively communicating what is happening to the user who is logging in.

WilliamH
08/27/2014 01:28 PM by
WilliamH

How pessimistic he is about writing code i.e. code for the worst hope for the best.

Josh
08/27/2014 01:49 PM by
Josh

Whether they decide the "3rd Party" service is inadequate and create their own implementation? ;-)

Layle Baker
08/27/2014 03:13 PM by
Layle Baker

Looks like you are looking at how a developer would handle any abstract "end-point" in code. How to handle exceptions and long-running issues.

Ultimately, you want someone to code the consuming end to handle any implementation (exception and long-running tasks, for example) - basically, NO dependencies, except for the interface.

(Of course, in the long-run, you get that magical "Decoupling" that people like to yammer on about - EVEN BETTER though, you can get both sides of the development (consumers and provider) to have more cohesive design)

Valerio Borioni
08/27/2014 03:14 PM by
Valerio Borioni

Detecting and handling transient and external failures ?

Mike
08/27/2014 03:18 PM by
Mike

Here's things I would look for:

  • Should ask you what's up with "userId" parameter and how he sets or gets that when creting a user. It's not reasonable to assume a "Name" and an "Id" are the same thing, nor an "email".
  • Should catch and handle whatever exceptions the third party has documented (hah!) at a minimum.
  • Should catch and handle unknown exceptions gracefully.
  • Should criticize your poor hashing algorithm (but should also understand that it's just an interview question and doesn't need to be perfect). Bonus points for running a dictionary attack against whatever sample data you give them?
Joseph Daigle
08/27/2014 03:27 PM by
Joseph Daigle

That is an awesome interview question. You're testing so many different real world skills. I may have to borrow this for future candidates.

Gluber
08/27/2014 03:43 PM by
Gluber

The long timeout would be a big issue IMO.

Since the given API is delivered in library form ( and i suppose asking to access the underlying endpoint directly is out of bounds ) its quite hard to handle that properly.

The interface is sync ( i guess on purpose for the test ) so async/await is out of the question ( wrapping it into a task running on the thread pool is a bad idea if you want this to scale ).... and also does not give you a way to monitor timeouts.... so probably creating your own mini thread pool ( i am talking about having a bounded set of maybe 2-4 threads statically allocated ) that process these calls and allow you to monitor timing from the outside would be an option.

Layle Baker
08/27/2014 04:04 PM by
Layle Baker

@Gluber - well if it was a long-running task and WASN'T wrapped Task object, it still wouldn't scale either, would it? :)

Still have to handle it. Wrap it in a Task that could time out, and log timeout so it can be examine later ("hey this interface method implementation keeps running too long quite a bit, we'll need to address it")?

Gilligan
08/27/2014 05:29 PM by
Gilligan

I am not sure focusing on making it async is the right answer here. What is the user going to do until they are logged in? "Hey here are a bunch of useless pages for you to look at until you can actually look at the important stuff!" The only way I can think of to mitigate performance is to cache the user credentials (and tbh I don' t know if I would want to take on the responsibility of storing those in my system, even though the password is encrypted) I think retry logic and exception handling is way more interesting here for ayende to learn about the interviewee.

Gilligan
08/27/2014 05:30 PM by
Gilligan

Mitigate performance Issues*

Mike
08/27/2014 06:27 PM by
Mike

@Gilligan, I was thinking the same thing, but there are actually a few ways you could handle this. You might tell the user that his email address needs to be verified and he should wait for a confirmation email, which presumably only gets sent if the async task finishes. Even if it's bulllshit to work around your slow vendor, users are used to that kind of delay and get a lot less cranky about that than an apparently hung UI. Provided the email does actually get sent eventually.

Taher
08/27/2014 07:35 PM by
Taher

How the End User Experience is handled ?

Cristian
08/28/2014 07:45 AM by
Cristian

Release It!

Oğuzhan Eren
08/28/2014 09:00 AM by
Oğuzhan Eren

Write a wrapper for IUserService as composite, handle exception in that class

Gluber
08/28/2014 02:09 PM by
Gluber

@Layle

You're right obviously, but doing it ourselves gives us the option to easier stay bounded on the ammount of work we're doing for this one endpoint.

( we don't need the whole complicated work stealing scheduling etc )

Another option would be a custom task scheduler, but that is more work.

Cancelling it is nigh impossible though since just abandoning the thread etc would leave it in an uncertain state.

A. Crowley
08/29/2014 10:00 AM by
A. Crowley

Robust system integration: From how developer handle out of his control issues, exception handling, timeouts, async operations for retries to facade layer if needed.

Securty; Is the developer salting the password? Is the salt random and unique for each user? How and where is the developer storing the salt?

Control and validation of user imput. XSS, SQL Injenction, valid email format, undesired or not printable characters, etc

Domain rules: Already UserId (email) exist in the system?

Instrumentation: Trancing, logging, monitoring performance, diagnosing errors.

Test: Unit test, Integration Test.

Luc Bos
08/29/2014 10:19 AM by
Luc Bos

Tests!

Only way to make sure the implementation behaves as it is supposed to and that with future upgrades the implementation does not change. All the rests (error handling, timeouts, ect) will be covered by the tests.

Pure Krome
09/01/2014 01:57 PM by
Pure Krome

You are depending on a 3rd party service == expect AND handle all manner or problems that are unexpected.

Unit tests, unit tests and unit tests! The first clue is that first piece of code -> it's an interface :)

Start with the happy path. Fine, 1 test down. Now - tests for all the crap that occurs when u suddenly leave localhost.

For me, the interview test is about how the candidate thinks with respect to possible scenario's that could occur.

** Is the candidate is a lateral thinker?

In the famous words of the Boy Scouts Motto: BE PREPARED.

Adam Wright
09/11/2014 11:31 AM by
Adam Wright

I think one thing you would be looking for is does candidate read your blog? :) http://ayende.com/blog/159457/design-patterns-in-the-test-of-time-adapter

Other interesting programming related concepts are do they retry authentication when GetUser is unsuccessful(assuming the password hash matching wasn't the issue), do they have a strategy for retries, how long should you wait before telling the user that there might want to try again, or try again at a later time. There is probably only so much time the user would wait before leaving the app with a bad taste in their mouth, without an explanation of what to do or what is happening. Do you tell the user the status, like a service is unavailable? It is an interesting problem that most of us probably don't think about when writing 3rd party authentication code.

fbhdev
09/12/2014 09:10 PM by
fbhdev

I think what is looked for here is how to solve the performance issue without letting the user experience suffer. The way I would solve this would be by sending a command object to a message queue, then handling the command in a command handler by pulling the message off the queue, calling the IUserService, then notifying the user when the account is created (so they activate their account). That way, the user doesn't have a slow user experience, and user (error handling, retries) logic is encapsulated in another part of our code.

Comments have been closed on this topic.