Summary:	ASTERISK-04026: [patch] ENUM - properly handling of multiple NAPTR records
Reporter:	Edwin Groothuis (mavetju)	Labels:
Date Opened:	2005-05-02 02:42:08	Date Closed:	2011-06-07 14:05:07
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Applications/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) patch-apps::app_enumlookup.c ( 1) patch-enum.c ( 2) patch-apps::app_enumlookup.c
Description:	The current implementation of ENUM doesn't work if there are multiple NAPTR records with different priorities / orders: It always takes the first entry from the DNS answer. I have changed asterisk/enum.c a little bit and it now supports multiple NAPTR records and takes the order and preference into account. It works as follows: The first time EnumLookup() is called, it loads all NAPTR records in a linked list and returns the first entry (order and preference wise). Every next call to EnumLookup() returns the next one (order and preference wise). How long does this linked list exist? Each element in the list has the channel_uniqueid which asked for the lookup first and it has a timer and after 15 minutes they get cleaned up. Is 15 minutes a lot? Yes/No. Yes because if you do a lot of lookups, it can grow a bit. No because you have to take timeouts into account, so if you have four phone numbers in it, you need a timeout of say 4x45 seconds which already makes 3 minutes. I can probably set it to 5 minutes or so, it just has to be determined later. A new command called EnumLookupNG has been introduced. Its syntax will make it easier for people to use it and know where it is actually jumping to: EnumLookupNG(exten,#VOIP,#TEL,#MAILTO,#FAIL) See the comments at the begining of app_enumlookup.c for a full demonstration on how to use it. ****** ADDITIONAL INFORMATION ****** - Current patch only for HEAD. - Patch for 1.0.7 is coming. - Disclaimer on file.
Comments:	By: Olle Johansson (oej) 2005-05-02 11:11:12 We will not patch stable 1.0.7, so only patch for head is needed. THank you, I will look into this shortly! By: Olle Johansson (oej) 2005-05-02 11:13:13 The formatting of the source code is not right, please check code guidelines. You need to indent with a <tab>. Also, please add /*--- functionname: description */ headers on top of each new function to document what they do. By: Edwin Groothuis (mavetju) 2005-05-02 18:02:27 I have uploaded new patches which are according to the code guidelines. By: Olle Johansson (oej) 2005-05-03 01:23:04 Hmmm. I personally don't like the string of arguments to enumlookupng. I think we can copy dial()s return value that is used for a goto. What if we change enumlookup to return ${ENUMTECH} and then we can goto that? exten => 100, 1, enumlookup(${MYNUMBER}) exten => 100, 2, goto(enum-${ENUMTECH},1) exten => enum-TEL,1,noop exten => enum-SIP,1,noop exten => enum-MAILTO,1,noop That will be more in line with current dial plan behaviour, and also extensible. By: Edwin Groothuis (mavetju) 2005-05-03 18:29:25 I'll implement the ENUMTECH solution today, because that one indeed looks better. By: Kevin P. Fleming (kpfleming) 2005-05-04 16:46:07 Ugh... we obviously need a way for apps to hang some dynamic data off of the channel structure. That would simplify this a great deal. In the meantime, can I suggest an alternative implementation? Recently, decisions have been made to move away from apps that auto-jump to different priorities, and I think it makes sense to not move to different extensions either. If we break this down into smaller pieces, the user will have more control over what is done with the results. How about some new dialplan functions: - ENUM_LOOKUP(number) would do the lookup as now, and return back the highest-priority URI as a single string. It would also format all the other URIs and store them (strung together with some sort of delimiter) in a channel variable called 'ENUMDATA' - ENUM_TECH(URI) would split the technology part of a URI off and return it as a simple string - ENUM_NEXT() would return the first URI contained in 'ENUMDATA' and remove it from the variable, so the dialplan can loop through the available URIs This would allow the dialplan writer to easily pick from the available URIs by technology, if they so choose, without complicating the lookup syntax. It also allows them to completely control how the URI is handled. Thoughts? By: Edwin Groothuis (mavetju) 2005-05-04 17:17:30 The current implementation as I have it is this: ------------------- 8< --------------------- [Description]: EnumLookup(exten):  Looks up an extension via ENUM, sets the variables 'ENUMDEST', 'ENUMTECH' and 'ENUMDETAIL' (for VOIP). For VoIP URIs this variable will look like 'TECHNOLOGY/URI' with the appropriate technology. Returns -1 on hangup, or 0 on completion regardless of whether the lookup was successful. If the technology is TEL, ENUMTECH is set to TEL and ENUMDEST has the number to dial. If the technology is MAILTO, ENUMTECH is set to MAILTO and ENUMDEST has the email address. If the technology is SIP, H323, IAX or IAX2, ENUMTECH is set to VOIP, ENUMDETAIL is set to the technology and ENUMDEST has first parameter for the Dial command. If technology is unknown or something else, ENUMTECH is set to fail and ENUMDETAIL contains (hopefully) useful information about what happened. ------------------- 8< --------------------- This way you can decide in the enum-VOIP priority (see oej's proposal) if you support SIP, or H323, or IAX, or IAX2. The reason to have a seperate -VOIP and -TEL priority which both use Dial, is the difference in handling of the destinations. The only thing I'm not sure if I want to leave it in is the TECHNOLOGY/URI in the VOIP result, since the TEL result doesn't return Zap/xx in front of it neither and you can get the TECHNOLOGY via ENUMDETAIL (ENUMDEST = ENUMDETAIL/destination). With regarding to the statement of "it makes sense to not move to different extensions either.", with the current one (as in the description earlier in this comment), I really like the similarity with the result of Dial and the usage of DIALSTATUS. By: Edwin Groothuis (mavetju) 2005-05-04 18:53:51 05-04-05 18:52 patch implements the behaviour described in comment 0026820. By: Kevin P. Fleming (kpfleming) 2005-05-04 20:49:26 Well, I wasn't so concerned about the channel variables holding the results, as I was about the data being kept in the app with an expiration timer and the extra code required to support that. I'm also concerned that a subsequent ENUMLookup in the same channel would _not_ do a new lookup, even if that's what the user wants to do. In other words, it's important to not try to do "too much" in the application, but rather to let the dialplan writer focus on interpreting the results as they see fit. For example, setting a policy that VOIP-like technologies get reported as 'VOIP' instead of their native name seems like a bad idea to me... As far as similarity to Dial, keep in mind that we have decided to stop using the n+101-type priority changes in apps, and focus on storing results in channel variables so the dialplan writer can (again) control the behavior explicitly. Finally, a decision was made during last weeks' dev conf call to stop adding/enhancing apps that don't actually interact with the channel, and start converting them over to dialplan functions (see the recent changes for MD5 and GROUP_COUNT). This application is a prime candidate for that switchover, so any significant changes to it should very seriously consider moving in that direction. Do you understand my proposal using three ENUM-related functions and how that would work? Can you see any reason why that would _not_ work? By: Edwin Groothuis (mavetju) 2005-05-04 21:09:39 > As far as similarity to Dial, keep in mind that we have decided to stop using > the n+101-type priority changes in apps, and focus on storing results in channel > variables so the dialplan writer can (again) control the behavior explicitly. I was talking about the DIALSTATUS, not about the implicit jump to current priority+n. In all my proposals with regarding to this feature there has not been a single implicit priority jump, they were always defined by the dialplan. I'll see if I can get your proposal in code. > Finally, a decision was made during last weeks' dev conf call to stop > adding/enhancing apps that don't actually interact with the channel, and > start converting them over to dialplan functions (see the recent changes for > MD5 and GROUP_COUNT). This application is a prime candidate for that > switchover, so any significant changes to it should very seriously consider > moving in that direction. I have no idea what you are talking about. Was there a discussion on -dev about this? I've read this paragraph seven times now and still don't know what the expected path is :-) By: Kevin P. Fleming (kpfleming) 2005-05-04 21:25:44 Sorry for being unclear... My references to dialplan jumps were because the ENUMLookup app already does that, but I can see with your last version that you've dropped that...that's good. However, I don't think we could break people's dialplans that way, so if we do modify ENUMLookup() itself it has to be less invasive somehow. No, the conversation about converting to functions was not on asterisk-dev directly, although it did get brought up there. Basically, the idea is to eliminate applications that only set/get variables or do other simple functionality, but don't actually interact with the channel in any significant way. The preferred form of these tools now is 'functions', as seen in pbx.c in CVS HEAD. At the moment we don't have a good place for these to go as loadable modules, but I'm working on that. Functions can take arguments, so it would be possible to provide filtering criteria to an ENUM_LOOKUP() function if the user wanted to limit the results to only particular technologies up-front (or anything else that makes sense). They can also be "writable" as if they were variables, although I don't see that would apply here. I was hoping that you'd be able to help me determine if my proposal would actually be usable in the dialplan in the way you envisioned yours being... but if you want to implement it first and then try it out, that's OK too of course. By: Edwin Groothuis (mavetju) 2005-05-04 21:47:47 > At the moment we don't have a good place for these to go as loadable modules, > but I'm working on that. Aha, that is what confused me: From your reply it showed that I understood it, but because there was no evidence of any other function in the tree like there is in apps/ I couldn't link the two together. Right now, until the new storage place and API is there, I keep using apps/ and the apps-API. > I was hoping that you'd be able to help me determine if my proposal would > actually be usable in the dialplan If it wasn't better approach than what there is now (inherited from the original EnumLookup) I would have shot it into a million pieces. It's still standing there ;-) > However, I don't think we could break people's dialplans that way That's why I have called it EnumLookupNG. By: Kevin P. Fleming (kpfleming) 2005-05-15 02:20:58 OK, as you may have seen, the CVS HEAD tree now has a bunch of functions that have replaced applications, and many more are in the works. It is easy to build a simple loadable module for them, or you can choose to have them built into pbx_functions.so (but I would suggest func_enum.so for these functions). Now that the infrastructure is in place, can we talk about implementing this new functionality entirely in terms of dialplan functions? By: Michael Jerris (mikej) 2005-05-25 12:55:06 Status update?  Were you able to take a look at the functions? By: Tilghman Lesher (tilghman) 2005-06-26 13:41:15 Closed due to lack of response by the reporter.  Reopen if you have a function implementation to upload.