yolk / valvat

Validates european vat numbers. Standalone or as a ActiveModel validator.
MIT License
318 stars 80 forks source link

More modular valvat gem (Feature request - clarification) #36

Closed duksis closed 1 year ago

duksis commented 9 years ago

First of all thanks for your awesome work!

I would like to ask the maintainers what do they think about making the gem more modular, by what I mean splitting it in multiple smaller gems that come together in the "valvat" just like "rspec" does it.

use case: At the moment I have a project where I need to validate the syntax of VAT numbers without the need to verify them against web services. And one of the constraints of the project is that I cant include the "savon" gem.

As this is used in the lookup and I don't need to do any lookups it would be grate if I could require just the parts I need (syntax, utils and maybe validations).

Would splitting the gem be something that the maintainers would consider?

Regards, Hugo Duksis

yolk commented 9 years ago

Hi @duksis, I absolutly understand your usecase, but I am a little bit worried this change would add a lot of complexity for only a little gain. But I will leave this issue open; maybe someone else is interested in this and wants to jump in.

Fivell commented 9 years ago

+1 for extracting sort of "valvat_validation" from valvat gem

dmitry commented 9 years ago

@Fivell but why, it's adding when ActiveModel persisted in the project (https://github.com/yolk/valvat/blob/master/lib/valvat.rb#L58), and ignored when not. If you are using own version of valvat validator in your project it can be done by the naming this validation differently. Separate gem just to decouple and make it heavy-less for some projects, where no validation required? I don't see any real point in extraction just the validation.

Maybe it would be good to have somekind of a settings that will give possibility to setup required parts of the system and remove forced requirement of all the parts in https://github.com/yolk/valvat/blob/master/lib/valvat.rb.

jgrau commented 7 years ago

I am looking at the memory used when requiring the different gems in my Gemfile. I also use valvat only for syntax validation - not actually checking against the webservice - for that reason it's offputting that valvat uses 3.5MiB of which savon is the 3.

Maybe Valvat::Lookup could have it's own interface. I mean, if you remove

def exists?(options={})
  Valvat::Lookup.validate(self, options)
end

from valvat.rb you could get by NOT requiring Lookup and thereby not require savon. Existing users of exists? could then use Valvat::Lookup.validate('foo') instead.

yolk commented 7 years ago

@jgrau I just added the possibility to load valvat without lookup/savon. I hasitated to change the normal usage/interface. You need to require 'valvat/local' instead of 'valvat'.

Does this change solve your problem?

jgrau commented 7 years ago

Oh yes. That's perfect. Even better than my solution.

yolk commented 7 years ago

@jgrau Cool! Just released it with v0.7.1

I am going to close this issue for now... even if it is only partly resolved. I don't want to eat the complexity of separate gems for verification & lookup.

jgrau commented 7 years ago

IMO it's fully resolved. Thanks!

edzhelyov commented 7 years ago

@yolk kudos man, and the approach of not forcing savon as required dependency is a good trade-off. 👏

edzhelyov commented 7 years ago

Hm, I still see that you require savon as gem dependency. I can suggest that you remove it from the gemspec and use it in the Gemfile during the development phrase, but that will require people to include two gems if they want to use web services.

jgrau commented 7 years ago

@edzhelyov It's true that the gem is still required but if you "gem 'valvat', :require => 'valvat/local'". in the Gemfile then it's never loaded and should take up memory. I believe @yolk don't want to force people to change their implementation when updating the gem version which would be the case if you removed savon as a gem dependency. That said, my usecase would also benefit from your solution...

yolk commented 7 years ago

@edzhelyov I will keep this in mind ... maybe for a new major version. Reopening the ticket...

sos4nt commented 2 years ago

@yolk just wanted to let you know that I would also prefer to have a Savon free variant as the project's current state is a little uncertain: https://github.com/savonrb/savon/issues/904

What about extracting the local / core part(s) from valvat into another gem valvat-local / valvat-core. That way, valvat could depend on both, savon and valvat-local / valvat-core and nothing would change for existing users.

jonian commented 2 years ago

Maybe savon can be removed and net/http can be used like in europe gem as you can see at https://github.com/gem-shards/europe.rb/blob/3622ce494fb10918a8413b827351d345f5c9a963/lib/europe/vat/vat.rb.

yolk commented 2 years ago

Thanks for the suggestion. Yes, this would make a much leaner valvat possible. I am a little bit unsure if this wouldn't contradict the main purpose of SOAP as a self-describing API. But I'll take a look into it...

jonian commented 2 years ago

Another option is to use a lightweight SOAP helper like lolsoap that does not have many dependencies, only nokogiri.

# faraday can be replaced by net/http

require 'faraday'
require 'lolsoap'

VIES_WSDL_URL = 'https://ec.europa.eu/taxation_customs/vies/checkVatService.wsdl'

wsdl    = Faraday.get(VIES_WSDL_URL).body
client  = LolSoap::Client.new(wsdl)
request = client.request('checkVat')

request.body do |b|
  b.countryCode 'EL'
  b.vatNumber '123456789'
end

http = Faraday.new(request.url)
resp = http.post do |req|
  req.headers = request.headers
  req.body    = request.content
end

response = client.response(soap_request, resp.body)
response.body_hash
yolk commented 1 year ago

@jonian: I implement the nethttp-solution in the nethttp branch.

Maybe you could take a look at it? Would like to release it as valvat 1.2 soon.

jonian commented 1 year ago

@yolk, looks great! Was the best solution to have zero dependencies. Thank you for your work!

yolk commented 1 year ago

Thanks @jonian

I just released v1.2 and gonna close this issue – after eight years! Savon is not a dependency anymore; and the lookup much leaner.