Merge pull request #131 from pacoorozco/issue-122

Fix import zone tool

app/Console/Commands/ProBINDImportZone.php

@@ -9,60 +9,36 @@
 
 class ProBINDImportZone extends Command
 {
-    /**
-     * The name and signature of the console command.
-     *
-     * @var string
-     */
+    const SUCCESS_CODE = 0;
+    const ERROR_PARSING_FILE_CODE = 1;
+    const ERROR_EXISTING_ZONE_CODE = 2;
+
     protected $signature = 'probind:import
                 {--domain= : The zone domain name to create}
-                {--file= : The file name to import}
-                {--force : Delete existing zone before import}';
-
-    /**
-     * The console command description.
-     *
-     * @var string
-     */
-    protected $description = 'Import a BIND zone file to ProBIND';
+                {--file= : The file name to import}';
 
-    /**
-     * Create a new command instance.
-     */
-    public function __construct()
-    {
-        parent::__construct();
-    }
+    protected $description = 'Imports a BIND zone file to ProBIND';
 
-    /**
-     * Execute the console command.
-     */
-    public function handle(): void
+    public function handle(): int
     {
-        // Cast supplied arguments and options.
-        $domain = (string) $this->option('domain');
-        $filename = (string) $this->option('file');
+        $domain = $this->ensureFQDN($this->option('domain'));
 
-        // Adds the ending '.' (dot) to the zone name.
-        $domain = (substr($domain, -1) != '.') ? $domain . '.' : $domain;
+        if (Zone::where('domain', $domain)->first()) {
+            $this->error('Zone can not be imported. A zone for the provided domain already exists.');
 
-        if (! $this->option('force')) {
-            // Check if Zone exists on database.
-            $existingZone = Zone::where('domain', $domain)->first();
+            return self::ERROR_EXISTING_ZONE_CODE;
+        }
 
-            if ($existingZone) {
-                $this->error('Zone \'' . $existingZone->domain . '\' exists on ProBIND. Use \'--force\' option if you want to import this zone.');
+        try {
+            $zoneData = $this->parseFile($domain, $this->option('file'));
+        } catch (\Throwable $exception) {
+            $this->error('The provided file could not be parsed.');
 
-                return;
-            }
+            return self::ERROR_PARSING_FILE_CODE;
         }
 
-        // Delete zone, if exists on database.
-        $this->deleteZoneIfExists($domain);
-
-        $zoneData = $this->parseFile($domain, $filename);
+        /** @var Zone $zone */
         $zone = Zone::create([
-            'custom_settings' => true,
             'domain' => $domain,
             'reverse_zone' => Zone::isReverseZoneName($domain),
         ]);
@@ -79,6 +55,7 @@ public function handle(): void
                 ]);
                 continue;
             }
+
             $zone->records()->create([
                 'name' => $record->getName(),
                 'ttl' => $record->getTtl(),
@@ -88,43 +65,28 @@ public function handle(): void
             $createdRecordsCount++;
         }
 
-        $this->info('Import zone \'' . $domain . '\' has created with ' . $createdRecordsCount . ' imported records.');
-        activity()->log('Import zone <strong>' . $zone->domain . '</strong> has created <strong>' . $createdRecordsCount . '</strong> records.');
+        $this->info('A zone for ' . $domain . ' domain has been created. ' . $createdRecordsCount . ' records has been imported.');
+        activity()->log('Created zone <strong>' . $zone->domain . '</strong> by importing <strong>' . $createdRecordsCount . '</strong> records.');
+
+        return self::SUCCESS_CODE;
     }
 
-    /**
-     * Delete the specified zone by domain search if exists.
-     *
-     * @param string $domain
-     */
-    private function deleteZoneIfExists(string $domain): void
+    private function ensureFQDN(string $domain): string
     {
-        // Check if Zone exists on database, including trashed zones.
-        $existingZone = Zone::withTrashed()
-            ->where('domain', $domain)->first();
-
-        if ($existingZone) {
-            $existingZone->forceDelete();
-        }
+        return (substr($domain, -1) != '.') ? $domain . '.' : $domain;
     }
 
     /**
      * Parses a DNS zone file and returns its content.
      *
-     * @param string $domain
-     * @param string $filename
+     * @param  string  $domain
+     * @param  string  $filename
      *
      * @return \Badcow\DNS\Zone
-     * @throws \ErrorException
+     * @throws \Badcow\DNS\Parser\ParseException
      */
     private function parseFile(string $domain, string $filename): \Badcow\DNS\Zone
     {
-        try {
-            $file = file_get_contents($filename);
-
-            return Parser\Parser::parse($domain, $file);
-        } catch (\Exception $exception) {
-            throw new \ErrorException();
-        }
+        return Parser\Parser::parse($domain, file_get_contents($filename));
     }
 }

app/Http/Controllers/ToolsController.php

@@ -21,6 +21,8 @@
 use App\Server;
 use App\Zone;
 use Illuminate\Support\Facades\Artisan;
+use Illuminate\Support\Facades\Storage;
+use Illuminate\View\View;
 
 class ToolsController extends Controller
 {
@@ -103,36 +105,18 @@ public function importZone()
         return view('tools.import_zone');
     }
 
-    /**
-     * Call Artisan 'probind:import' command with supplied data.
-     *
-     * @param ImportZoneRequest $request
-     *
-     * @return \Illuminate\Http\RedirectResponse
-     *
-     * FIXME
-     * Use queues for doing long tasks on background. This will require some notification system.
-     * Errors processing import Zone.
-     */
-    public function importZonePost(ImportZoneRequest $request)
+    public function importZoneFromFile(ImportZoneRequest $request): View
     {
-        // Move uploaded file to local storage.
-        $zonefile = $request->file('zonefile')->store('temp');
-        if (false === $zonefile) {
-            // Handle error
-            redirect()->route('home')
-                ->with('error',
-                    __('tools/messages.import_zone_error', ['zone' => $request->input('domain')]));
-        }
+        $filename = $request->zoneFile()->store('temp');
 
         Artisan::call('probind:import', [
-            'zone'     => $request->input('domain'),
-            'zonefile' => storage_path('app/' . $zonefile),
-            '--force'  => $request->has('overwrite'),
+            '--domain' => $request->domain(),
+            '--file' => Storage::path($filename),
         ]);
 
-        return redirect()->route('home')
-            ->with('success',
-                __('tools/messages.import_zone_success', ['zone' => $request->input('domain')]));
+        Storage::delete($filename);
+
+        return view('tools.import_zone_result')
+            ->with('output', Artisan::output());
     }
 }

app/Http/Requests/ImportZoneRequest.php

@@ -3,30 +3,39 @@
 namespace App\Http\Requests;
 
 use Illuminate\Foundation\Http\FormRequest;
+use Illuminate\Http\UploadedFile;
+use Illuminate\Validation\Rule;
 
 class ImportZoneRequest extends FormRequest
 {
-    /**
-     * Determine if the user is authorized to make this request.
-     *
-     * @return bool
-     */
-    public function authorize()
+    public function authorize(): bool
     {
         return true;
     }
 
-    /**
-     * Get the validation rules that apply to the request.
-     *
-     * @return array
-     */
-    public function rules()
+    public function rules(): array
     {
         return [
-            'domain'    => 'required|string',
-            'zonefile'  => 'required|file|max:2048',
-            'overwrite' => 'sometimes|boolean',
+            'domain' => [
+                'required',
+                'string',
+                Rule::unique('zones'),
+            ],
+            'zonefile' => [
+                'required',
+                'mimetypes:text/plain',
+                'max:2048',
+            ],
         ];
     }
+
+    public function domain(): string
+    {
+        return $this->input('domain');
+    }
+
+    public function zoneFile(): ?UploadedFile
+    {
+        return $this->file('zonefile');
+    }
 }

resources/views/tools/import_zone.blade.php

@@ -57,6 +57,7 @@
                         <span class="help-block">{{ $errors->first('domain', ':message') }}</span>
                     </div>
                 </div>
+
             </div>
 
             <div class="box-footer">

resources/views/tools/import_zone_result.blade.php

@@ -0,0 +1,60 @@
+@extends('layouts.admin')
+
+{{-- Web site Title --}}
+@section('title', __('tools/title.push_updates'))
+
+{{-- Content Header --}}
+@section('header')
+    {{ __('tools/title.import_zone') }}
+    <small>{{ __('tools/title.import_zone_subtitle') }}</small>
+@endsection
+
+{{-- Breadcrumbs --}}
+@section('breadcrumbs')
+    <li>
+        <a href="{{ route('home') }}">
+            <i class="fa fa-dashboard"></i> {{ __('site.dashboard') }}
+        </a>
+    </li>
+    <li>
+        <a href="{{-- route('tools.index') --}}">
+            {{ __('site.tools') }}
+        </a>
+    </li>
+    <li class="active">
+        {{ __('tools/title.import_zone') }}
+    </li>
+@endsection
+
+{{-- Content --}}
+@section('content')
+    <div class="box">
+        <div class="box-header with-border">
+            <h3 class="box-title">{{ __('tools/title.import_zone') }}</h3>
+        </div>
+
+        <div class="box-body">
+            <div class="form-group">
+                <textarea class="form-control" rows="15" id="output" disabled>{{ $output }}</textarea>
+            </div>
+        </div>
+
+        <div class="box-footer">
+            <a href="{{ route('home') }}" class="btn btn-primary pull-right" role="button">
+                {{ __('general.done') }} <i class="fa fa-arrow-right"></i>
+            </a>
+        </div>
+
+    </div>
+@endsection
+
+{{-- Scripts --}}
+@push('scripts')
+    <script>
+        $(function () {
+            var $textarea = $('#output');
+            $textarea.scrollTop($textarea[0].scrollHeight);
+        });
+    </script>
+@endpush
+

routes/web.php

@@ -105,7 +105,7 @@
 Route::get('tools/import',
     ['as' => 'tools.import_zone', 'uses' => 'ToolsController@importZone']);
 Route::post('tools/import',
-    ['as' => 'tools.import_zone_post', 'uses' => 'ToolsController@importZonePost']);
+    ['as' => 'tools.import_zone_post', 'uses' => 'ToolsController@importZoneFromFile']);
 /*  ------------------------------------------
  *  Settings
  *  ------------------------------------------